From 6ab2061f7246c0611ca327f9a0993a4cbb267d20 Mon Sep 17 00:00:00 2001 From: freewil Date: Thu, 18 Dec 2025 11:16:13 +0000 Subject: [PATCH] get_info memleak fix: cache exception string only * get_info(): Cache only the exception message * Create fresh exceptions * No traceback accumulation, no memory leak, cleaner code Caching the exception objects for `address` and `fingerprint` get_info() params and re-raising them creates an accumulation of python frame and traceback objects, essentially creating a memory leak with observable linear growth of memory usage over time. This appears to be an anti-pattern and prevents the python garbage collector from collecting these exceptions and also creates references to other objects, preventing collection. Instead of caching the entire exception object, this commit just caches the exception string, ensuring no unintentional references are created that accumulate frame, tracebacks, or other application objects. --- stem/control.py | 22 +++++++++++----------- test/unit/control/controller.py | 20 ++++++++++---------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/stem/control.py b/stem/control.py index d2494ca9e..b32bb7ba3 100644 --- a/stem/control.py +++ b/stem/control.py @@ -1063,8 +1063,8 @@ def __init__(self, control_socket, is_authenticated = False): self._enabled_features = [] self._is_geoip_unavailable = None - self._last_address_exc = None - self._last_fingerprint_exc = None + self._last_address_error = None + self._last_fingerprint_error = None super(Controller, self).__init__(control_socket, is_authenticated) @@ -1094,7 +1094,7 @@ def _address_changed_listener(event): if event.action in ('EXTERNAL_ADDRESS', 'DNS_USELESS'): self._set_cache({'exit_policy': None}) self._set_cache({'address': None}, 'getinfo') - self._last_address_exc = None + self._last_address_error = None self.add_event_listener(_address_changed_listener, EventType.STATUS_SERVER) @@ -1179,10 +1179,10 @@ def get_info(self, params, default = UNDEFINED, get_bytes = False): for param in params: if param.startswith('ip-to-country/') and param != 'ip-to-country/0.0.0.0' and self.is_geoip_unavailable(): raise stem.ProtocolError('Tor geoip database is unavailable') - elif param == 'address' and self._last_address_exc: - raise self._last_address_exc # we already know we can't resolve an address - elif param == 'fingerprint' and self._last_fingerprint_exc and self.get_conf('ORPort', None) is None: - raise self._last_fingerprint_exc # we already know we're not a relay + elif param == 'address' and self._last_address_error: + raise stem.ControllerError(self._last_address_error) # we already know we can't resolve an address + elif param == 'fingerprint' and self._last_fingerprint_error and self.get_conf('ORPort', None) is None: + raise stem.ControllerError(self._last_fingerprint_error) # we already know we're not a relay # check for cached results @@ -1230,10 +1230,10 @@ def get_info(self, params, default = UNDEFINED, get_bytes = False): self._set_cache(to_cache, 'getinfo') if 'address' in params: - self._last_address_exc = None + self._last_address_error = None if 'fingerprint' in params: - self._last_fingerprint_exc = None + self._last_fingerprint_error = None log.debug('GETINFO %s (runtime: %0.4f)' % (' '.join(params), time.time() - start_time)) @@ -1243,10 +1243,10 @@ def get_info(self, params, default = UNDEFINED, get_bytes = False): return list(reply.values())[0] except stem.ControllerError as exc: if 'address' in params: - self._last_address_exc = exc + self._last_address_error = str(exc) if 'fingerprint' in params: - self._last_fingerprint_exc = exc + self._last_fingerprint_error = str(exc) log.debug('GETINFO %s (failed: %s)' % (' '.join(params), exc)) raise diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py index 94c1b65fb..2aa790114 100644 --- a/test/unit/control/controller.py +++ b/test/unit/control/controller.py @@ -72,20 +72,20 @@ def test_get_info(self, msg_mock): def test_get_info_address_caching(self, msg_mock): msg_mock.return_value = ControlMessage.from_str('551 Address unknown\r\n') - self.assertEqual(None, self.controller._last_address_exc) - self.assertRaisesWith(stem.OperationFailed, 'Address unknown', self.controller.get_info, 'address') - self.assertEqual('Address unknown', str(self.controller._last_address_exc)) + self.assertEqual(None, self.controller._last_address_error) + self.assertRaisesWith(stem.ControllerError, 'Address unknown', self.controller.get_info, 'address') + self.assertEqual('Address unknown', self.controller._last_address_error) self.assertEqual(1, msg_mock.call_count) # now that we have a cached failure we should provide that back - self.assertRaisesWith(stem.OperationFailed, 'Address unknown', self.controller.get_info, 'address') + self.assertRaisesWith(stem.ControllerError, 'Address unknown', self.controller.get_info, 'address') self.assertEqual(1, msg_mock.call_count) # invalidates the cache, transitioning from no address to having one msg_mock.return_value = ControlMessage.from_str('250-address=17.2.89.80\r\n250 OK\r\n', 'GETINFO') - self.assertRaisesWith(stem.OperationFailed, 'Address unknown', self.controller.get_info, 'address') + self.assertRaisesWith(stem.ControllerError, 'Address unknown', self.controller.get_info, 'address') self.controller._handle_event(ControlMessage.from_str('650 STATUS_SERVER NOTICE EXTERNAL_ADDRESS ADDRESS=17.2.89.80 METHOD=DIRSERV\r\n')) self.assertEqual('17.2.89.80', self.controller.get_info('address')) @@ -102,20 +102,20 @@ def test_get_info_without_fingerprint(self, get_conf_mock, msg_mock): msg_mock.return_value = ControlMessage.from_str('551 Not running in server mode\r\n') get_conf_mock.return_value = None - self.assertEqual(None, self.controller._last_fingerprint_exc) - self.assertRaisesWith(stem.OperationFailed, 'Not running in server mode', self.controller.get_info, 'fingerprint') - self.assertEqual('Not running in server mode', str(self.controller._last_fingerprint_exc)) + self.assertEqual(None, self.controller._last_fingerprint_error) + self.assertRaisesWith(stem.ControllerError, 'Not running in server mode', self.controller.get_info, 'fingerprint') + self.assertEqual('Not running in server mode', self.controller._last_fingerprint_error) self.assertEqual(1, msg_mock.call_count) # now that we have a cached failure we should provide that back - self.assertRaisesWith(stem.OperationFailed, 'Not running in server mode', self.controller.get_info, 'fingerprint') + self.assertRaisesWith(stem.ControllerError, 'Not running in server mode', self.controller.get_info, 'fingerprint') self.assertEqual(1, msg_mock.call_count) # ... but if we become a relay we'll call it again get_conf_mock.return_value = '443' - self.assertRaisesWith(stem.OperationFailed, 'Not running in server mode', self.controller.get_info, 'fingerprint') + self.assertRaisesWith(stem.ControllerError, 'Not running in server mode', self.controller.get_info, 'fingerprint') self.assertEqual(2, msg_mock.call_count) @patch('stem.control.Controller.get_info')