Merge pull request #10737 from sashazykov/hw-close-wallet-thread-leak
What the developer wrote
Authored by Felix
Merge pull request #10737 from sashazykov/hw-close-wallet-thread-leak
hw_wallet: fix crash on exit if device unpairing fails
What changed, and why it matters
This commit fixes a shutdown bug in Electrum's hardware wallet support. Previously, if disconnecting from a hardware wallet failed, the cleanup thread could keep running, which could crash the application when closing. The fix stops the thread before attempting disconnect, and makes the disconnect step tolerate errors instead of crashing.
Apply the patch. It is a defensive hardening fix for a crash-on-exit condition and does not require urgent incident response, but users relying on hardware wallets should update to avoid abnormal shutdowns.
Security signals we found
Process abort/crash at shutdown due to leaked QThread
Exception during cleanup not handled, potentially causing abnormal termination
Hardware wallet client close made best-effort with error logging
Evidence from the diff
The patch reorders close_wallet() in electrum/hw_wallet/plugin.py so that keystore.thread.stop() is called before device_manager().unpair_pairing_code(). The commit message explains that if unpairing raises, a still-running QThread would cause Qt abort() at shutdown. It also wraps client.close() in electrum/plugin.py with a try/except to log failures rather than propagate exceptions during cleanup.
Changed components
electrum/hw_wallet/plugin.pyelectrum/plugin.pyHardware wallet plugin shutdown pathDeviceManager client cleanupInspect 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
@@ -1132,7 +1132,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 32/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.