tests: clear util.callback_mgr between test cases
What the developer wrote
Authored by SomberNight
tests: clear util.callback_mgr between test cases
util.callback_mgr.callbacks was not getting properly cleared between tests.
Every time an Abstract_Wallet or an LNWorker (or many other subclasses of EventListener) is instantiated,
self.register_callbacks() is called in __init__, which puts callbacks into util.callback_mgr.callbacks.
These are only cleaned up if we explicitly call Abstract_Wallet.stop() or LNWorker.stop() later, which we usually do not do in the tests.
As a result, when running multiple unit tests in a row, lots of objects created in a given testcase are never GC-ed and leak into subsequent tests. This is not only a memory leak, but wastes compute too: when events are triggered and cbs get called, these old objects also have their cbs called.
After running all (~1061) unit tests, I observe util.callback_mgr.callbacks had 30 events with a total of 3156 callbacks stored.
On my laptop, running all unit tests previously took ~115 sec, and now it takes ~73 sec.
What changed, and why it matters
This commit is a test-suite cleanup, not a fix for a user-facing security bug. It stops old test objects from leaking between unit tests by clearing a global callback list after each test. That makes tests faster and less error-prone, but it does not change how the real Electrum wallet handles callbacks in production.
No user or operator action needed. This is a test-hygiene improvement. Reviewers may optionally verify that production code paths still call stop()/unregister_callback() appropriately so real wallets do not leak callbacks in long-running processes.
Security signals we found
Global callback registry leak between test cases
Test-only cleanup added; no production behavior change
No input validation, privilege boundary, or cryptographic change
Evidence from the diff
The change adds CallbackManager.clear_all_callbacks() and calls it in ElectrumTestCase.tearDown(). Previously, EventListener subclasses such as Abstract_Wallet and LNWorker registered callbacks in util.callback_mgr during init but were rarely stopped explicitly in tests, causing object leaks and wasted event dispatches. The patch also removes manual unregister_callback calls in test_lnpeer.py because tearDown now clears them globally. The production callback registration/unregistration logic is unchanged.
Changed components
electrum/util.py CallbackManagertests/__init__.py ElectrumTestCase.tearDowntests/test_lnpeer.pyInspect captured patch +13 / −16
diff --git a/electrum/util.py b/electrum/util.py
index b019b08..cc27336 100644
--- a/electrum/util.py
+++ b/electrum/util.py
@@ -1953,20 +1953,24 @@ class CallbackManager(Logger):
def __init__(self):
Logger.__init__(self)
self.callback_lock = threading.Lock()
- self.callbacks = defaultdict(list) # note: needs self.callback_lock
+ self.callbacks = defaultdict(list) # type: Dict[str, List[Callable]] # note: needs self.callback_lock
- def register_callback(self, func, events):
+ def register_callback(self, func: Callable, events: Sequence[str]) -> None:
with self.callback_lock:
for event in events:
self.callbacks[event].append(func)
- def unregister_callback(self, callback):
+ def unregister_callback(self, callback: Callable) -> None:
with self.callback_lock:
for callbacks in self.callbacks.values():
if callback in callbacks:
callbacks.remove(callback)
- def trigger_callback(self, event, *args):
+ def clear_all_callbacks(self) -> None:
+ with self.callback_lock:
+ self.callbacks.clear()
+
+ def trigger_callback(self, event: str, *args) -> None:
"""Trigger a callback with given arguments.
Can be called from any thread. The callback itself will get scheduled
on the event loop.
diff --git a/tests/__init__.py b/tests/__init__.py
index 663dc60..106323e 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -75,6 +75,7 @@ class ElectrumTestCase(unittest.IsolatedAsyncioTestCase, Logger):
util._asyncio_event_loop = loop
def tearDown(self):
+ util.callback_mgr.clear_all_callbacks()
shutil.rmtree(self.electrum_path)
super().tearDown()
util._asyncio_event_loop = None # cleared here, at the ~last possible moment. asyncTearDown is too early.
diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py
index 01b3074..b1bc6c5 100644
--- a/tests/test_lnpeer.py
+++ b/tests/test_lnpeer.py
@@ -1117,12 +1117,8 @@ class TestPeerDirect(TestPeer):
util.register_callback(on_htlc_fulfilled, ["htlc_fulfilled"])
util.register_callback(on_htlc_failed, ["htlc_failed"])
- try:
- with self.assertRaises(SuccessfulTest):
- await f()
- finally:
- util.unregister_callback(on_htlc_fulfilled)
- util.unregister_callback(on_htlc_failed)
+ with self.assertRaises(SuccessfulTest):
+ await f()
async def test_payment_recv_mpp_confusion2(self):
"""Regression test for https://github.com/spesmilo/electrum/security/advisories/GHSA-8r85-vp7r-hjxf"""
@@ -1191,12 +1187,8 @@ class TestPeerDirect(TestPeer):
util.register_callback(on_htlc_fulfilled, ["htlc_fulfilled"])
util.register_callback(on_htlc_failed, ["htlc_failed"])
- try:
- with self.assertRaises(SuccessfulTest):
- await f()
- finally:
- util.unregister_callback(on_htlc_fulfilled)
- util.unregister_callback(on_htlc_failed)
+ with self.assertRaises(SuccessfulTest):
+ await f()
async def test_legacy_shutdown_low(self):
await self._test_shutdown(alice_fee=100, bob_fee=150)
Why this scored 18/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.