hw_wallet: fix crash on exit if device unpairing fails
What the developer wrote
Authored by Sasha Zykov
hw_wallet: fix crash on exit if device unpairing fails
On wallet close, the close_wallet hook unpaired the device before
stopping the keystore TaskThread. Unpairing does device I/O and can
raise, e.g. if the device was unplugged while the wallet was open:
Plugin error. plugin: trezor, hook: close_wallet
Traceback (most recent call last):
File "electrum/plugin.py", line 833, in run_hook
r = f(*args)
File "electrum/hw_wallet/plugin.py", line 89, in close_wallet
self.device_manager().unpair_pairing_code(keystore.pairing_code())
File "electrum/plugin.py", line 1118, in unpair_pairing_code
self._close_client(_id)
File "electrum/plugin.py", line 1134, in _close_client
client.close()
...
File "electrum/plugins/trezor/clientbase.py", line 286, in close
self.client.lock()
...
trezorlib.transport.bridge.BridgeException: trezord: acquire/62/null failed with code 400: device not found
run_hook() swallows the exception, so the thread was never stopped.
A still-running QThread (child of the wallet window) at interpreter
shutdown then makes Qt abort the process:
QThread: Destroyed while thread '' is still running
Stop the thread before unpairing, and make DeviceMgr._close_client
treat client.close() as best-effort, as closing a missing device is
a normal condition during cleanup.
What changed, and why it matters
This commit fixes a crash that could happen when closing an Electrum wallet that uses a hardware device (like a Trezor). If the device was unplugged before closing, the wallet's cleanup step could fail and leave a background Qt thread running. At shutdown, Qt would then forcefully abort the whole program. The fix stops the thread first and treats device-close failures as harmless during cleanup.
Apply the patch. It is a low-risk defensive fix that prevents a reproducible crash during wallet shutdown when a hardware wallet is disconnected. No immediate incident response is required beyond normal patching.
Security signals we found
Denial-of-service via abnormal process termination (Qt abort) on wallet close
Unhandled exception in cleanup hook leading to resource leak (QThread)
Best-effort handling added for device I/O failures during teardown
Evidence from the diff
The patch reorders the close_wallet hook so the keystore TaskThread is stopped before device unpairing is attempted. It also wraps DeviceMgr._close_client’s client.close() in a try/except so that transport errors during cleanup (e.g., BridgeException when the Trezor was unplugged) are logged rather than propagated. Previously, run_hook swallowed the unpairing exception, so the thread stop was skipped, leaving a QThread alive and causing Qt to abort() at interpreter shutdown.
Changed components
electrum/hw_wallet/plugin.pyelectrum/plugin.pyHardware wallet keystore TaskThread lifecycleDeviceMgr client cleanup pathInspect captured patch +9 / −2
### electrum/hw_wallet/plugin.py
@@ -86,9 +86,11 @@ def create_device_from_hid_enumeration(self, d: dict, *, product_key) -> Optiona
def close_wallet(self, wallet: 'Abstract_Wallet'):
for keystore in wallet.get_keystores():
if isinstance(keystore, self.keystore_class):
- self.device_manager().unpair_pairing_code(keystore.pairing_code())
+ # stop the thread first: if unpairing raises, the thread must not be leaked,
+ # as a still-running QThread would make Qt abort() the process at shutdown
if keystore.thread:
keystore.thread.stop()
+ self.device_manager().unpair_pairing_code(keystore.pairing_code())
def get_client(self, keystore: 'Hardware_KeyStore', force_pair: bool = True, *,
devices: Sequence['Device'] = None,
### electrum/plugin.py
@@ -1131,7 +1131,12 @@ def _close_client(self, id_):
if fut := self._ongoing_timeout_checks.pop(id_, None):
fut.cancel()
if client:
- client.close()
+ try:
+ client.close()
+ except Exception as e:
+ # closing is best-effort: it does device I/O, which can fail,
+ # e.g. if the device was unplugged
+ self.logger.info(f"failed to close hardware client cleanly: {e!r}")
def _client_by_id(self, id_) -> Optional['HardwareClientBase']:
with self.lock:Why this scored 29/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.