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