From ff90824247885edf9ebf715d29a22a98ffcfc733 Mon Sep 17 00:00:00 2001 From: SamiAhmed7777 <79177212+SamiAhmed7777@users.noreply.github.com> Date: Tue, 7 Jul 2026 18:56:21 -0700 Subject: [PATCH] fix(wallet): prevent exit-hang on Windows from detached Tor/I2P threads (#20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(wallet): prevent exit-hang on Windows from detached Tor/I2P threads Embedded Tor and embedded I2P each ran on a background std::thread that was .detach()'d at startup. The teardown paths (CTorEmbedded::Stop, CI2PEmbedded::Stop) only flipped a running-flag — they did not signal the thread to exit, and on Windows there is no signal mechanism in tor_api 0.4.x. Result on Windows: when the user closed the wallet, Shutdown() completed its bookkeeping and main() returned 0, but the process could not exit because the detached thread was still in the Tor event loop / i2pd io_context. End Task (TerminateProcess) was the only escape; the GUI appeared completely stuck. Fixes: - tor_embedded.h/.cpp: keep the Tor thread handle; Stop() now raise(SIGTERM) on Linux, then joins the thread with a 5s timeout, then TerminateThread (Win) / pthread_cancel + pthread_join (Linux) as a last resort. - i2p_embedded.h/.cpp: same pattern — capture the bootstrap thread and join it in Stop() with a 5s timeout fallback. - init.cpp Shutdown(): spawn a 30s watchdog thread that calls ExitProcess(1) if the graceful teardown takes too long. Belt-and-suspenders against any future deadlock in the exit path. - trianglesgui.cpp closeEvent(): second close attempt while the first exit is still running immediately calls ExitProcess(2) / _exit(2). User escape hatch when the graceful exit hangs. All non-consensus (threading/process lifecycle only). Build via CI; not local. Notes: notes/wallet-close-hang-fix-2026-07-07.md * fix(i2p): drop leftover .detach() that broke build (lambda now joinable) * fix(i2p): clean up after .detach() removal (trailing comment, blank line) * fix(tor): MINGW std::thread is pthread-based, use pthread_cancel/join on MINGW MINGW std::thread::native_handle_type is unsigned long long (pthread_t emulation), not HANDLE. Mixing pthread handles with Win32 WaitForSingleObject/TerminateThread fails to compile on MINGW with 'invalid conversion' errors. Use the same pthread_cancel/pthread_join path on Linux and MINGW; keep TerminateThread only for MSVC builds where native_handle() returns a real Win32 HANDLE. --------- Co-authored-by: krystie --- notes/wallet-close-hang-fix-2026-07-07.md | 82 +++++++++++++++++++++++ src/i2p/i2p_embedded.cpp | 29 +++++++- src/i2p/i2p_embedded.h | 3 + src/init.cpp | 18 +++++ src/qt/trianglesgui.cpp | 16 ++++- src/tor/tor_embedded.cpp | 70 ++++++++++++++++--- src/tor/tor_embedded.h | 7 +- 7 files changed, 211 insertions(+), 14 deletions(-) create mode 100644 notes/wallet-close-hang-fix-2026-07-07.md diff --git a/notes/wallet-close-hang-fix-2026-07-07.md b/notes/wallet-close-hang-fix-2026-07-07.md new file mode 100644 index 0000000..85a377a --- /dev/null +++ b/notes/wallet-close-hang-fix-2026-07-07.md @@ -0,0 +1,82 @@ +# Wallet close hang fix (2026-07-07) + +**Reported by:** Sami +**Branch:** TBD (off `ui/overview-color-rework` or new `fix/close-hang`) +**Severity:** High — wallet process can't be closed by user on Windows +**Consensus-affecting:** No (threading/process lifecycle only) + +## Symptom + +- User clicks X on Qt wallet +- Wallet appears to hang +- Task Manager → End Task does not close the process (on Windows) +- No new `debug.log` output after the click + +## Root cause (Phase 1) + +`src/tor/tor_embedded.cpp:205-206`: + +```cpp +std::thread torThread(TorThreadFunc, argv); +torThread.detach(); +``` + +The Tor thread is **detached** at startup and never joined. `CTorEmbedded::Stop()` at line 266-278 only flips a `running` atomic — it has no real teardown on either platform: + +- **Linux:** `#ifndef WIN32` block is a no-op (comment-only TODO) +- **Windows:** no block at all — function body ends after `running.store(false)` + +`tor_run_main()` blocks in the Tor event loop indefinitely. The OS process **cannot exit** while that thread is alive, regardless of `main()` returning 0. `Shutdown()` in `init.cpp` finishes its bookkeeping and sets `fExit = true`, `main()` returns, but the process keeps running because the detached Tor thread is still in the event loop. + +`ExitTimeout` (init.cpp:143) is only useful for deadlock *after* `Shutdown()` returns — it doesn't help here. + +## Fix plan + +### 1. `src/tor/tor_embedded.cpp` — actually stop the Tor thread + +Two-part fix: + +a) Store the `std::thread` handle (not detached): +```cpp +std::thread torThread(TorThreadFunc, argv); +// do NOT detach +torThreadHandle = std::move(torThread); +``` + +b) In `Stop()`, on the **main thread**, send a Tor control command to ask the daemon to shut down. Tor's `tor_api` doesn't expose this in 0.4.x but the embedded Tor opens a control port by default OR we can use the simpler approach: send `SIGTERM` to ourselves on Linux, and on Windows post a custom event to the Tor thread. + +For Windows specifically: the cleanest approach is to use Tor's `tor_api_shutdown()` if available, OR fall back to `TerminateThread` after a 5-second grace period. Since the wallet is going to exit anyway, `TerminateThread` is acceptable here as a last-resort — we mark the thread as unjoinable and let OS clean it up. + +### 2. `src/i2p/i2p_embedded.cpp` — same fix for I2P + +I2P has a cleaner API: `i2p::api::StopI2P()` and `i2p::api::TerminateI2P()` exist (line 643, 646). The background thread in the lambda at line 527 is also detached. Same fix pattern: capture the thread handle, join it (with a 5s timeout fallback) in `Stop()`. + +### 3. `src/init.cpp` `Shutdown()` — add an overall watchdog + +Wrap the shutdown sequence in a timed watchdog. If `Shutdown()` doesn't return within 30 seconds, log where it got stuck and call `ExitProcess(0)` (Windows) / `_exit(0)` (Linux) to force-exit. This is the belt-and-suspenders that ensures the wallet ALWAYS closes, even if the I2P/Tor stop is partially broken in a future release. + +### 4. Belt-and-suspenders: pre-`Shutdown` user signal handler + +Add a `WM_CLOSE` handler that, on second close attempt (when one is already in progress), immediately force-exits. This is a UX improvement so users with a stuck wallet can force-close via X. + +## Files to modify + +- `src/tor/tor_embedded.cpp` (Stop() implementation) +- `src/tor/tor_embedded.h` (thread member + Stop() signature) +- `src/i2p/i2p_embedded.cpp` (Stop() implementation, thread capture) +- `src/i2p/i2p_embedded.h` (thread member) +- `src/init.cpp` (Shutdown() watchdog, force-exit on timeout) + +## Test plan + +1. Build CI green +2. Manual Windows test: open wallet, wait for Tor/I2P ready, close, verify < 5s shutdown +3. Manual Windows test: open wallet, immediately close, verify no hang +4. Manual Linux test: same as #2, verify clean exit +5. Stress test: open + close 5 times in a row, no resource leak + +## Risk + +- `TerminateThread` on Tor is unsafe but happens only on graceful timeout path +- The watchdog `_exit(0)` skips destructors; acceptable because the wallet is exiting anyway +- i2pd internals may have already-closed state; guarded with try/catch diff --git a/src/i2p/i2p_embedded.cpp b/src/i2p/i2p_embedded.cpp index 189c53d..4071f32 100644 --- a/src/i2p/i2p_embedded.cpp +++ b/src/i2p/i2p_embedded.cpp @@ -524,7 +524,9 @@ bool CI2PEmbedded::Start(int socks, int sam, int server) printf("Embedded I2P: launching router in background thread...\n"); fflush(stdout); - std::thread([this]() { + // Keep the thread handle so Stop() can join it. A detached thread + // that is still running would block the wallet from exiting. + routerThread = std::thread([this]() { try { // Start the I2P router (netdb, transports, tunnels, reseed) auto logStream = std::make_shared(std::cout.rdbuf()); @@ -615,7 +617,7 @@ bool CI2PEmbedded::Start(int socks, int sam, int server) printf("ERROR: Embedded I2P background init failed: %s\n", e.what()); fflush(stdout); } - }).detach(); + }); printf("Embedded I2P: router init delegated to background thread\n"); fflush(stdout); @@ -632,7 +634,10 @@ bool CI2PEmbedded::Start(int socks, int sam, int server) void CI2PEmbedded::Stop() { - if (!running.load()) return; + if (!running.load()) { + if (routerThread.joinable()) routerThread.join(); + return; + } printf("Requesting embedded I2P shutdown...\n"); try { @@ -648,6 +653,24 @@ void CI2PEmbedded::Stop() printf("WARNING: error during I2P shutdown: %s\n", e.what()); } + // Wait for the bootstrap thread to finish (up to 5s). + auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (routerThread.joinable() && std::chrono::steady_clock::now() < deadline) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + if (!running.load()) { + // The thread observes fShutdown and exits its loop on its own + // once running is set false by the API teardown above. + routerThread.join(); + break; + } + } + if (routerThread.joinable()) { + printf("WARNING: embedded I2P did not exit within 5s; detaching thread\n"); + // Detach as a last resort — the process is about to exit and the OS + // will reap the thread. + routerThread.detach(); + } + running.store(false); } diff --git a/src/i2p/i2p_embedded.h b/src/i2p/i2p_embedded.h index 6e0d0b1..107d74d 100644 --- a/src/i2p/i2p_embedded.h +++ b/src/i2p/i2p_embedded.h @@ -94,6 +94,9 @@ private: std::string i2pDataDir; // i2pd data directory (under wallet datadir) std::string i2pHostname; // Our .b32.i2p address (available after router startup) std::string lastError; + // I2P bootstrap runs in a background thread; we keep the handle so Stop() + // can join it. (A detached thread that is still running blocks process exit.) + std::thread routerThread; public: static CI2PEmbedded* GetInstance(); diff --git a/src/init.cpp b/src/init.cpp index e64994c..9b89670 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -333,6 +333,24 @@ void Shutdown(void* parg) // Make this thread recognisable as the shutdown thread RenameThread("Triangles-shutoff"); + // Belt-and-suspenders: spawn a watchdog that force-exits if Shutdown() + // doesn't complete in 30 seconds. This protects against deadlock in the + // embedded Tor/I2P teardown paths (see notes/wallet-close-hang-fix-2026-07-07.md). + std::thread([]() + { +#ifdef WIN32 + Sleep(30000); + fprintf(stderr, "Shutdown watchdog: 30s elapsed, force-exiting process\n"); + fflush(stderr); + ExitProcess(1); +#else + sleep(30); + fprintf(stderr, "Shutdown watchdog: 30s elapsed, force-exiting process\n"); + fflush(stderr); + _exit(1); +#endif + }).detach(); + bool fFirstThread = false; { TRY_LOCK(cs_Shutdown, lockShutdown); diff --git a/src/qt/trianglesgui.cpp b/src/qt/trianglesgui.cpp index 9f7d214..abd35ac 100644 --- a/src/qt/trianglesgui.cpp +++ b/src/qt/trianglesgui.cpp @@ -91,7 +91,7 @@ #include #include - +#include #include extern std::unique_ptr pwalletMain; extern int64_t nLastCoinStakeSearchInterval; @@ -1084,6 +1084,20 @@ void TrianglesGUI::closeEvent(QCloseEvent *event) } } #endif + // Second close attempt while the first shutdown is still in progress: + // force-exit immediately. This is the user's "get me out" path when the + // graceful shutdown is taking too long (e.g. embedded Tor/I2P teardown + // is stuck). See notes/wallet-close-hang-fix-2026-07-07.md. + static std::atomic fShuttingDown(false); + if (fShuttingDown.exchange(true)) { + fprintf(stderr, "TrianglesGUI::closeEvent: second close while shutting down, force-exit\n"); + fflush(stderr); +#ifdef WIN32 + ExitProcess(2); +#else + _exit(2); +#endif + } // Actually closing - request a full core shutdown before leaving the UI loop. StartShutdown(); event->accept(); diff --git a/src/tor/tor_embedded.cpp b/src/tor/tor_embedded.cpp index 9b1f4d2..f53588b 100644 --- a/src/tor/tor_embedded.cpp +++ b/src/tor/tor_embedded.cpp @@ -20,14 +20,17 @@ #include #include #include +#include #ifdef WIN32 #include #include +#include #else #include #include #include +#include #endif #ifdef ENABLE_TOR_EMBEDDED @@ -202,8 +205,10 @@ bool CTorEmbedded::Start(int socks, int hsPort, bool enableHiddenService) running.store(true); // Launch Tor on a dedicated thread (tor_run_main blocks) - std::thread torThread(TorThreadFunc, argv); - torThread.detach(); + // We keep the handle (do NOT detach) so Stop() can join the thread. + // A detached thread that is still running will block process exit + // indefinitely on both Windows and Linux. + torThread = std::thread(TorThreadFunc, argv); // Wait for SOCKS port to become available (up to 60s) printf("Waiting for embedded Tor to bootstrap...\n"); @@ -265,15 +270,62 @@ bool CTorEmbedded::Start(int socks, int hsPort, bool enableHiddenService) void CTorEmbedded::Stop() { - if (!running.load()) return; + if (!running.load()) { + // Not running; just make sure the thread handle is released + if (torThread.joinable()) torThread.join(); + return; + } printf("Requesting embedded Tor shutdown...\n"); - // tor_run_main respects signals; raise SIGTERM to trigger graceful exit -#ifndef WIN32 - // On Unix we can signal our own process; the Tor thread handles it - // Actually, tor_api doesn't provide a clean shutdown function in 0.4.x - // For now, the thread will exit when the process exits. - // TODO: Tor 0.4.9+ may add tor_api_shutdown(), use it when available + + // tor_run_main respects signals; raise SIGTERM to trigger graceful exit. + // On Linux this causes tor_run_main to return and the thread to exit. + // On Windows there is no signal mechanism in tor_api 0.4.x — we have to + // wait for tor_run_main to return on its own (the shutdown path is + // triggered by the `running` flag being observed by the calling code, + // but tor_run_main itself does not poll it). In practice Tor exits when + // the process exits, so we just join with a timeout below. +#if !defined(WIN32) || defined(__MINGW32__) + // MINGW std::thread is pthread-based, so signal-based shutdown works + // there too. Raise SIGTERM so tor_run_main can observe it. + raise(SIGTERM); #endif + + // Give Tor up to 5 seconds to shut down cleanly. + auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (torThread.joinable() && std::chrono::steady_clock::now() < deadline) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + if (!running.load()) { + // Tor's TorThreadFunc sets running=false after tor_run_main returns. + torThread.join(); + break; + } + } + + if (torThread.joinable()) { + // Tor did not exit cleanly within 5 seconds. Force-terminate the + // thread as a last resort — we are about to exit the process anyway. + printf("WARNING: embedded Tor did not exit within 5s; force-terminating thread\n"); +#if defined(WIN32) && !defined(__MINGW32__) + // MSVC std::thread::native_handle_type is HANDLE (a pointer) on + // Windows. TerminateThread is unsafe but the process is exiting. + TerminateThread(torThread.native_handle(), 0); + WaitForSingleObject(torThread.native_handle(), 1000); + // After TerminateThread the handle is still valid; detach to release. + torThread.detach(); +#else + // Linux + MINGW: std::thread is pthread-based, native_handle() returns + // pthread_t. pthread_cancel is async: the thread exits at its next + // cancellation point (or immediately for C code with no cancellation + // points — in which case pthread_join blocks). Either way, + // pthread_join drains the thread. Detach the std::thread handle so the + // destructor doesn't call std::terminate on a still-joinable handle. + pthread_cancel(torThread.native_handle()); + void* retval = nullptr; + pthread_join(torThread.native_handle(), &retval); + torThread.detach(); +#endif + } + running.store(false); } diff --git a/src/tor/tor_embedded.h b/src/tor/tor_embedded.h index 91f1590..06d5a53 100644 --- a/src/tor/tor_embedded.h +++ b/src/tor/tor_embedded.h @@ -20,6 +20,10 @@ private: std::string torDataDir; std::string onionHostname; std::string lastError; + // Tor runs in a background thread; we keep the handle so Stop() can + // join it instead of leaking a detached thread that blocks process exit. + // (Detached + still running = OS refuses to exit the process.) + std::thread torThread; public: static CTorEmbedded* GetInstance(); @@ -30,7 +34,8 @@ public: // Start Tor in a background thread (blocks that thread until shutdown) bool Start(int socksPort = 19099, int hsPort = 24112, bool enableHiddenService = true); - // Request Tor to shut down + // Request Tor to shut down and join the background thread (with timeout). + // Safe to call multiple times. void Stop(); // Check if Tor is running and bootstrapped