util.CallbackManager: follow-up prev: fix deadlock
What the developer wrote
Authored by SomberNight
util.CallbackManager: follow-up prev: fix deadlock
What changed, and why it matters
This commit changes an internal lock in Electrum's callback system from a regular lock to a re-entrant lock (RLock) and adds a comment explaining that the lock must be re-entrant because object cleanup (garbage collection) can now trigger code that tries to acquire the same lock again. A regular lock would cause a 'deadlock'—the program freezing while waiting for itself. The commit message frames this as a follow-up fix for a previous deadlock fix. It is a reliability/availability issue rather than a direct theft-of-funds bug, but deadlocks in wallet software can make the application unresponsive.
Treat as a stability/reliability fix. Review the prior commit this follows up on to confirm the full deadlock path is addressed. Consider adding a regression test that exercises callback cleanup under garbage collection to prevent future regressions. No immediate emergency response is warranted unless the prior commit is found to introduce a more severe vulnerability.
Security signals we found
Deadlock / denial-of-service in wallet GUI/daemon due to lock ordering issue
Use of re-entrant lock to safely handle __del__ re-entry
Follow-up to previous deadlock fix, indicating prior concurrency bug
No explicit security framing by vendor in commit message
Evidence from the diff
In electrum/util.py, CallbackManager.init now initializes callback_lock as threading.RLock instead of threading.Lock. A comment is added inside unregister_callback noting that callback_lock must be re-entrant because the code path can trigger del, which also takes callback_lock. This prevents self-deadlock when unregister_callback holds the lock and garbage collection of a weakly-referenced callback invokes a destructor that re-enters the same lock. The change is defensive and small, but the commit does not include a regression test or reproduction case.
Changed components
electrum/util.py:CallbackManagercallback registration/unregistration subsystemweakref-based callback cleanup pathsInspect captured patch +2 / −1
diff --git a/electrum/util.py b/electrum/util.py
index 72ff594..9736f54 100644
--- a/electrum/util.py
+++ b/electrum/util.py
@@ -1954,7 +1954,7 @@ class CallbackManager(Logger):
def __init__(self):
Logger.__init__(self)
- self.callback_lock = threading.Lock()
+ self.callback_lock = threading.RLock()
self._wcallbacks = defaultdict(set) # type: Dict[str, Set[weakref.ref[Callable]]] # note: needs self.callback_lock
@staticmethod
@@ -1976,6 +1976,7 @@ class CallbackManager(Logger):
def unregister_callback(self, cb: Callable) -> None:
wcb = self._wcb_from_any_callback(cb)
with self.callback_lock:
+ # note: ^ callback_lock needs to be re-entrant, as we can now trigger __del__, which also takes the lock
for callbacks in self._wcallbacks.values():
if wcb in callbacks:
callbacks.remove(wcb)
Why this scored 41/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.