plugin: make DeviceMgr.run non-blocking, fix lock
What the developer wrote
Authored by f321x
plugin: make DeviceMgr.run non-blocking, fix lock
Prevents `DeviceMgr.run()` from blocking the `Plugins` `DaemonThread` by
scheduling the hww timeout check instead of awaiting its result on the
`Plugins` thread.
If something in the `_hwd_comms_executor` thread is waiting for user
input, e.g. when setting up a hww in the wizard the user needs to
unlock the hww for `HardwareClientBase.get_xpub()` to return, the
`_hwd_comms_executor` is blocked. If then `DeviceMgr.run()` gets called by
the `Plugins` `DaemonThread` concurrently and tries to check the hww
timeout on the `_hwd_comms_executor` as well the `DaemonThread` is
blocked too until the `_hwd_comms_executor` gets unblocked (and the
`DaemonThread.job_lock` is taken.
Now if something tries to take the `DaemonThread.job_lock` it blocks as
well, so if a user e.g. tries to load a new plugin from the plugins
dialog the whole gui thread will freeze until the hww gets unlocked.
What changed, and why it matters
This commit fixes a bug where Electrum's background plugin thread could freeze while waiting for a hardware wallet to be unlocked. The fix moves the timeout check onto a separate executor thread so the main background thread stays responsive. A side effect is that the GUI no longer freezes when loading plugins while a hardware wallet is waiting for user input. There is no direct evidence this is a security vulnerability, but thread-blocking bugs can sometimes be abused to create denial-of-service conditions.
Treat as a stability and UX fix. Review whether the blocking behavior could be triggered by an attacker with local access or a malicious plugin; if so, consider a security advisory. Otherwise, include in regular release notes as a bug fix.
Security signals we found
Fixes thread-blocking / lock contention issue
DaemonThread.job_lock contention could freeze GUI
Potential local denial-of-service via hardware wallet user interaction
No explicit security disclosure in commit message
Evidence from the diff
DeviceMgr.run() previously called client.timeout(cutoff) directly on the Plugins DaemonThread. If the _hwd_comms_executor thread was blocked waiting for user input (e.g., hardware wallet unlock), a concurrent DeviceMgr.run() call could also submit work to that executor and block the DaemonThread, which holds DaemonThread.job_lock. Any other code needing job_lock (such as plugin loading) would then block, freezing the GUI. The patch schedules timeout checks via _hwd_comms_executor.submit() and tracks in-flight Future objects per client, canceling them on client removal. It also adds logging in DaemonThread.run() to detect long-blocking thread jobs.
Changed components
electrum/plugin.pyelectrum/util.pyInspect captured patch +17 / −3
diff --git a/electrum/plugin.py b/electrum/plugin.py
index 021df3e..3a7892b 100644
--- a/electrum/plugin.py
+++ b/electrum/plugin.py
@@ -36,6 +36,7 @@ from urllib.parse import urlparse
from typing import (NamedTuple, Any, Union, TYPE_CHECKING, Optional, Tuple,
Dict, Iterable, List, Sequence, Callable, TypeVar, Mapping)
import concurrent
+from concurrent.futures import Future
import zipimport
from functools import wraps, partial
from itertools import chain
@@ -1040,6 +1041,7 @@ class DeviceMgr(ThreadJob):
self._recognised_vendor = {} # type: Dict[int, HW_PluginBase] # vendor_id -> Plugin
# Custom enumerate functions for devices we don't know about.
self._enumerate_func = set() # Needs self.lock.
+ self._ongoing_timeout_checks = {} # type: Dict[str, Future]
self.lock = threading.RLock()
@@ -1053,10 +1055,16 @@ class DeviceMgr(ThreadJob):
"""Handle device timeouts. Runs in the context of the Plugins
thread."""
with self.lock:
- clients = list(self.clients.keys())
+ clients = list(self.clients.items())
cutoff = time.time() - self.config.get_session_timeout()
- for client in clients:
- client.timeout(cutoff)
+ for client, client_id in clients:
+ if fut := self._ongoing_timeout_checks.get(client_id):
+ if not fut.done():
+ continue
+ # scheduling the timeout check prevents blocking the Plugins DaemonThread if the
+ # _hwd_comms_executor Thread is blocked (e.g. due to it awaiting user input).
+ fut = _hwd_comms_executor.submit(client.timeout, cutoff)
+ self._ongoing_timeout_checks[client_id] = fut
def register_devices(self, device_pairs, *, plugin: 'HW_PluginBase'):
for pair in device_pairs:
@@ -1113,6 +1121,8 @@ class DeviceMgr(ThreadJob):
with self.lock:
client = self._client_by_id(id_)
self.clients.pop(client, None)
+ if fut := self._ongoing_timeout_checks.pop(id_, None):
+ fut.cancel()
if client:
client.close()
diff --git a/electrum/util.py b/electrum/util.py
index 5223d85..300a410 100644
--- a/electrum/util.py
+++ b/electrum/util.py
@@ -378,10 +378,14 @@ class DaemonThread(threading.Thread, Logger):
# malformed or malicious server responses
with self.job_lock:
for job in self.jobs:
+ start = time.perf_counter()
try:
job.run()
except Exception as e:
self.logger.exception('')
+ duration = time.perf_counter() - start
+ if duration > 0.5:
+ self.logger.warning(f"thread job {job} blocked {self} DaemonThread for {duration:.2f} s")
def remove_jobs(self, jobs):
with self.job_lock:
Why this scored 46/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.