Fix Electrum hanging on shutdown: cancel-safe, resumable announcement proof-of-work - #2
Merged
Conversation
… PoW
Electrum never fully shut down once the swap server had been enabled. The
asyncio loop runs on a non-daemon thread ('EventLoop', electrum/util.py), so
anything that blocks that thread keeps the interpreter alive forever.
Root cause: electrum.util.gen_nostr_ann_pow deadlocks the loop thread when the
awaiting task is cancelled mid-grind.
with ProcessPoolExecutor(max_workers=max_workers) as executor:
...
done, pending = await asyncio.wait(tasks, ...) # CancelledError
executor.shutdown(wait=False, cancel_futures=True) # success path only
On cancellation the context manager's __exit__ runs executor.shutdown(wait=True),
a synchronous join, on the loop thread. The workers only exit when a shared
shutdown Event is set, and that Event is only set by a worker that *finds* a
solution, so the join never returns.
The plugin reached that code from two tasks it cancels at shutdown:
run_nostr_server and _one_shot_publish both begin with set_nostr_proof_of_work.
stop_server() cancels both from the close_wallet / on_close_window hooks. With
the default 30-bit target and nonce 0 a fresh server always grinds (~3.5 min on
an 8-core box), so quitting in that window wedged the loop permanently. The two
tasks also ground concurrently, doubling CPU use for identical work.
Fix: own the proof of work (plugins/swapserver_gui/pow.py) and seed the nonce
before starting the nostr transport, so upstream's set_nostr_proof_of_work
always short-circuits on a cached nonce and never enters the broken function.
* raw multiprocessing.Process instead of ProcessPoolExecutor, whose atexit
hook is itself a hang vector; cancellation signals, terminates and hands
joining to a daemon thread, never blocking the loop or the GUI thread
* resumable: workers checkpoint their nonce cursor, persisted per lane in a
new config var, so a grind cancelled by quitting continues into untested
nonce space instead of re-scanning the range upstream deterministically
re-tests. Tracking the best nonce seen keeps cursors valid for any target
* thread fallback where fork is unavailable (Windows/Android), since the
plugin package is not guaranteed importable in a spawned child
Also fixed, all contributing to an unclean teardown:
* stop_server() now unregisters the HTTP EventListener synchronously, so a
cancelled cleanup coroutine cannot leave stale callbacks registered
* the 4s refresh QTimer is stopped when the tab is removed; it was calling
into get_asyncio_loop() while the loop was being torn down
* publish_now waits on a PoW gate rather than computing its own
The tab now shows proof-of-work progress and an estimated grind time next to
the target, and confirms before raising the target past 30 bits.
Tests: 52 pass. tests/test_shutdown_e2e.py reproduces the reported bug end to
end, including a subprocess test asserting a whole interpreter actually exits
while the proof of work is still running, and that the wallet closes cleanly
mid-grind with progress preserved. Verified against the real upstream helper:
cancelling gen_nostr_ann_pow wedges the loop and the process needs SIGKILL,
while the replacement exits cleanly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013zETFC6MhnsCuxvQDGxqV5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Electrum never fully shut down once the swap server had been enabled — the window closed but the process stayed alive.
Electrum's asyncio loop runs on a non-daemon thread (
EventLoop,electrum/util.py:1710), so anything that blocks that thread keeps the interpreter alive forever. Shutdown also goes throughasyncio.run_coroutine_threadsafe(self.stop(), ...).result()— blocking, no timeout (electrum/daemon.py:694) — so a wedged loop freezes the GUI thread too.Root cause
electrum.util.gen_nostr_ann_powdeadlocks the loop thread when the awaiting task is cancelled mid-grind:On cancellation the context manager's
__exit__runsexecutor.shutdown(wait=True)— a synchronous join, on the event loop thread. The workers (util.py:2381) only exit when a sharedshutdownEvent is set, and that Event is only set by a worker that finds a solution. The join never returns.This plugin reached that code from two tasks it cancels at shutdown:
run_nostr_serverand_one_shot_publishboth begin withset_nostr_proof_of_work, andstop_server()cancels both from theclose_wallet/on_close_windowhooks.SWAPSERVER_POW_TARGETdefaults to 30 bits andSWAPSERVER_ANN_POW_NONCEto 0 (which scores 0 bits), so a fresh server always grinds — measured ~3.5 min expected on an 8-core box, and exponentially distributed, so 8–10 min is unremarkable. Quitting anywhere in that window wedged the loop permanently. The two tasks also ground concurrently, each spawningcpu_count-1workers plus aManagerprocess, doubling CPU use for identical work.Verified before/after, against the real upstream function
Same harness, cancel a grind after 4s, then probe whether the loop can still run
asyncio.sleep(0):gen_nostr_ann_powWEDGEDpow.grindOKThe fix
Own the proof of work (
plugins/swapserver_gui/pow.py) and seed the nonce before starting the nostr transport, so upstream'sset_nostr_proof_of_workalways short-circuits on a cached nonce and never enters the broken function.multiprocessing.Processinstead ofProcessPoolExecutor, whoseatexithook joins workers at interpreter shutdown — itself a hang vector. Cancellation signals,terminate()s, and hands joining to a daemon thread, so it never blocks the loop or the GUI thread.forkis unavailable (Windows/Android). The plugin package isn't guaranteed importable in aspawned child — it may load from a zip aselectrum_external_plugins.swapserver_gui— so a process backend there would fail at unpickling. Single-core but correct and cancel-safe; CI's 2-core runners exercise this path.Also fixed (contributing to unclean teardown)
stop_server()unregisters the HTTPEventListenersynchronously — the cleanup coroutine was fire-and-forget and could be cancelled by the loop shutting down, leaving stale callbacks registered against a stopped wallet.QTimeris now stopped when the tab is removed; it kept calling intoget_asyncio_loop()while the loop was being torn down.publish_nowwaits on a PoW gate rather than computing its own, so an offer is never announced with a nonce takers would reject.UI
The tab shows proof-of-work progress (best bits so far / hashes scanned — no fake percentage, since the search is memoryless) and an estimated grind time next to the target, measured on the local machine. Raising the target past 30 bits now asks for confirmation: 35 bits is ~2 hours and 40 bits ~2.5 days on an 8-core box.
Testing
52 tests pass under
unittest discover, as CI runs.tests/test_shutdown_e2e.pyreproduces the reported bug end to end, driving the real plugin through Electrum's actual shutdown sequence (non-daemonEventLoopthread,stopping_fut, cancel-everything teardown):stop_server()doesn't block the GUI thread (< 2s).result()with no timeout fromdaemon.py:694returns — the precise call that hangs in the fieldtests/test_pow.pycovers cancel-safety on both backends, worker reaping, resume-past-cursor, state invalidation on wallet switch, and rejection of hand-edited/corrupt state.Known upstream issue, not addressed here
NostrTransport.__exit__(submarine_swaps.py:1798) blocks the loop ~5s on every cancel viafut.result(timeout=5)called from the loop thread. Bounded and no longer fatal once the deadlock is gone; documented in a comment rather than worked around.🤖 Generated with Claude Code
https://claude.ai/code/session_013zETFC6MhnsCuxvQDGxqV5