util.CallbackManager: use sets instead of lists
What the developer wrote
Authored by SomberNight
util.CallbackManager: use sets instead of lists
- to gracefully take duplicate calls of register_callbacks(): should be idempotent now
- as a side-effect, the order of the callbacks is changed and not guaranteed
- not like anyone should have been relying on it before though
What changed, and why it matters
This commit changes an internal event-notification system in Electrum so that registering the same callback multiple times is now harmless (idempotent) instead of creating duplicates. It also adds a docstring warning that failing to unregister callbacks can leak memory. There is no direct evidence of an exploitable security vulnerability in the diff itself; it reads as a robustness improvement.
Treat as a routine robustness/cleanup patch. Review callers of EventListener to ensure unregister_callbacks() is consistently invoked, as the newly added docstring highlights a pre-existing memory-leak risk. No urgent security deployment is indicated by this commit alone.
Security signals we found
memory-leak documentation note added for EventListener lifecycle
duplicate callback registration now idempotent, reducing risk of accidental callback amplification
no input validation or trust-boundary changes visible in diff
Evidence from the diff
CallbackManager in electrum/util.py switches from defaultdict(list) to defaultdict(set), making register_callback() use set.add() instead of list.append(). This prevents duplicate callback registrations when register_callbacks() is called more than once. The iteration in trigger_callback() now copies the set. A docstring note was added to EventListener warning about memory leaks if unregister_callbacks() is omitted. The test expectations were updated to reflect idempotent registration counts.
Changed components
electrum/util.py:CallbackManagerelectrum/util.py:EventListenertests/test_callbackmgr.pyInspect captured patch +10 / −6
diff --git a/electrum/util.py b/electrum/util.py
index 5223d85..45608bd 100644
--- a/electrum/util.py
+++ b/electrum/util.py
@@ -21,6 +21,7 @@
# CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
# SOFTWARE.
import concurrent.futures
+import copy
from dataclasses import dataclass
import logging
import os
@@ -1953,12 +1954,12 @@ class CallbackManager(Logger):
def __init__(self):
Logger.__init__(self)
self.callback_lock = threading.Lock()
- self.callbacks = defaultdict(list) # type: Dict[str, List[Callable]] # note: needs self.callback_lock
+ self.callbacks = defaultdict(set) # type: Dict[str, Set[Callable]] # note: needs self.callback_lock
def register_callback(self, func: Callable, events: Sequence[str]) -> None:
with self.callback_lock:
for event in events:
- self.callbacks[event].append(func)
+ self.callbacks[event].add(func)
def unregister_callback(self, callback: Callable) -> None:
with self.callback_lock:
@@ -1978,7 +1979,7 @@ class CallbackManager(Logger):
loop = get_asyncio_loop()
assert loop.is_running(), "event loop not running"
with self.callback_lock:
- callbacks = self.callbacks[event][:]
+ callbacks = copy.copy(self.callbacks[event])
for callback in callbacks:
if inspect.iscoroutinefunction(callback): # async cb
fut = asyncio.run_coroutine_threadsafe(callback(*args), loop)
@@ -2004,8 +2005,11 @@ _event_listeners = defaultdict(set) # type: Dict[str, Set[str]]
class EventListener:
"""Use as a mixin for a class that has methods to be triggered on events.
- Methods that receive the callbacks should be named "on_event_*" and decorated with @event_listener.
- - register_callbacks() should be called exactly once per instance of EventListener, e.g. in __init__
+ - register_callbacks() should be called once per instance of EventListener, e.g. in __init__
- unregister_callbacks() should be called at least once, e.g. when the instance is destroyed
+ - if register_callbacks() is called in __init__, as opposed to a separate start() method,
+ extra care is needed that the call to unregister_callbacks() is not forgotten,
+ otherwise we will leak memory
"""
def _list_callbacks(self):
diff --git a/tests/test_callbackmgr.py b/tests/test_callbackmgr.py
index 1a390e7..cd5cd10 100644
--- a/tests/test_callbackmgr.py
+++ b/tests/test_callbackmgr.py
@@ -49,13 +49,13 @@ class TestCallbackMgr(ElectrumTestCase):
el2.start()
self.assertEqual(4, _count_all_callbacks())
el1.start()
- self.assertEqual(6, _count_all_callbacks())
- el1.stop()
self.assertEqual(4, _count_all_callbacks())
el1.stop()
self.assertEqual(2, _count_all_callbacks())
el1.stop()
self.assertEqual(2, _count_all_callbacks())
+ el1.stop()
+ self.assertEqual(2, _count_all_callbacks())
el2.stop()
self.assertEqual(0, _count_all_callbacks())
Why this scored 23/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.