From d9dae45c3907fca7a8a0c82f84fb976874faf86b Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 14 Oct 2026 07:51:34 -0500 Subject: [PATCH] fix(lite): flush the wallet save on quit before _Exit On quit the lite controller's destructor never runs, because main() ends the process with _Exit(). Any state the background refresh had scanned since the last save was lost: the destructor is where that final save would normally happen, and it was being skipped on the one path that always runs. - shutdown() now calls a new flushOnExit() that persists once on the way out, bounded to 4 s so the existing 15 s shutdown watchdog budget still holds. On timeout it abandons the save thread and lets _Exit follow. - It is a plain save through the same backend path, which writes a temp file and renames it into place. That cannot corrupt the wallet if it is cut off or abandoned, and it does not change the wallet file format - it only persists sooner on the quit path. - It reuses the destructor's guards and also skips when a wallet op is already in progress: shutdown() has already waited for ops to go idle, and a still-pending op holds the sync lock and issues its own save, so flushOnExit would only block the budget on a save that cannot take the lock. Tests cover a save on the open, synced, idle path (exactly once), a blocked save returning within the budget, and the retired and never-opened guards skipping the save. Co-Authored-By: Claude Opus 4.8 --- src/app.cpp | 3 ++ src/wallet/lite_wallet_controller.cpp | 28 ++++++++++++- src/wallet/lite_wallet_controller.h | 4 ++ tests/test_phase4.cpp | 58 +++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/app.cpp b/src/app.cpp index 7e12b0a..2c0f72d 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -7742,6 +7742,9 @@ void App::shutdown() // Lite: let an in-flight wallet op (e.g. Encrypt's save) finish. Bounded: the window is already hidden and // main.cpp has a 15 s watchdog. The backend writes tmp + rename, so being cut off never corrupts the wallet. if (lite_wallet_) lite_wallet_->waitWalletOpsIdle(std::chrono::milliseconds(2500)); + // Persist refresh-scanned state: ~LiteWalletController never runs on the _Exit path. Bounded (4 s) so the + // 15 s watchdog budget holds; the save is tmp + rename, so an abandoned/cut-off save never corrupts. + if (lite_wallet_) lite_wallet_->flushOnExit(std::chrono::milliseconds(4000)); // Full node: persist whatever a refresh changed since beginShutdown() (everything, on the direct-exit path), // then drain + join the history writer. Bounded like the settings flush: on a wedged disk the in-flight write diff --git a/src/wallet/lite_wallet_controller.cpp b/src/wallet/lite_wallet_controller.cpp index 445fe71..c8ce46c 100644 --- a/src/wallet/lite_wallet_controller.cpp +++ b/src/wallet/lite_wallet_controller.cpp @@ -11,7 +11,11 @@ #include #include +#include #include +#include +#include +#include #include #include @@ -499,6 +503,27 @@ LiteWalletController::LiteWalletController(WalletCapabilities capabilities, status_ = lifecycle_.status(); } +void LiteWalletController::flushOnExit(std::chrono::milliseconds budget) +{ + // Same guards as the destructor, plus walletOpInProgress(): App::shutdown() already ran + // waitWalletOpsIdle(); if an op is still pending it holds sync_lock and issues its own save, + // so skip rather than block the budget on a save that cannot acquire the lock. + if (retired_.load() || !walletOpen_.load() || !syncComplete() || + broadcastInProgress() || walletOpInProgress() || !bridge_) return; + struct Done { std::mutex m; std::condition_variable cv; bool done = false; }; + auto st = std::make_shared(); + auto bridge = bridge_; // shared_ptr copy; never captures `this` + try { + std::thread([bridge, st]() { + bridge->execute("save", ""); // may block on sync_lock; abandoned on timeout + { std::lock_guard l(st->m); st->done = true; } + st->cv.notify_all(); + }).detach(); + } catch (...) { return; } // no thread: skip (pre-fix behaviour, no crash) + std::unique_lock l(st->m); + st->cv.wait_for(l, budget, [&] { return st->done; }); // timeout: abandon the thread; _Exit follows +} + LiteWalletController::~LiteWalletController() { scan_->cancel.store(true); // the detached sync thread stops retrying (it may be in a back-off sleep) @@ -515,7 +540,8 @@ LiteWalletController::~LiteWalletController() } // The sync thread may be blocked in an uninterruptible full scan; detach it. It holds // shared refs (bridge_ + syncDone_), so it stays safe and the bridge survives until it - // finishes — the process is exiting, so a late litelib_shutdown is harmless. + // finishes — on the quit path the process is exiting, so a late litelib_shutdown is harmless + // (a server switch already shut the bridge down in retire()). if (syncThread_.joinable()) syncThread_.detach(); // Likewise the broadcast thread (send/shield proving): it captures shared refs (bridge + // running flag + result slot), never `this`, so detaching is safe. diff --git a/src/wallet/lite_wallet_controller.h b/src/wallet/lite_wallet_controller.h index 48914d9..e992e98 100644 --- a/src/wallet/lite_wallet_controller.h +++ b/src/wallet/lite_wallet_controller.h @@ -175,6 +175,10 @@ public: ~LiteWalletController(); // stops + joins the background refresh worker + // Best-effort persist on the quit path (the destructor's save is skipped because main() _Exit()s). + // Bounded: abandons the save thread on timeout; the process is about to _Exit. Same guards as ~dtor. + void flushOnExit(std::chrono::milliseconds budget); + LiteWalletController(const LiteWalletController&) = delete; LiteWalletController& operator=(const LiteWalletController&) = delete; diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index 9f27179..80d0e61 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -10427,6 +10427,63 @@ void testLiteWalletControllerM5Persistence() } } +// C30: flushOnExit() persists once on the quit path, is bounded when the save blocks, and obeys the guards. +void testLiteWalletControllerFlushOnExit() +{ + using namespace dragonx::wallet; + const auto liteCaps = makeWalletCapabilities(WalletBuildKind::Lite, false, true); + const LiteConnectionSettings conn = defaultLiteConnectionSettings(); + auto openSynced = [&]() { + dragonx::test::g_liteFakeSyncBlock = false; + auto c = std::make_unique( + liteCaps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi())); + LiteWalletCreateRequest req; + (void)c->createWallet(req); + for (int i = 0; i < 400 && !c->syncComplete(); ++i) + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + return c; + }; + + // (a) open + synced + idle -> exactly one save + { + auto c = openSynced(); + EXPECT_TRUE(c->syncComplete()); + const long before = dragonx::test::g_liteFakeSaveCount.load(); + c->flushOnExit(std::chrono::milliseconds(500)); + EXPECT_EQ(dragonx::test::g_liteFakeSaveCount.load(), before + 1); + } + + // (b) a blocked save never hangs the caller past the budget + { + auto c = openSynced(); + dragonx::test::g_liteFakeBlockSave = true; + const auto t0 = std::chrono::steady_clock::now(); + c->flushOnExit(std::chrono::milliseconds(200)); + const auto dt = std::chrono::steady_clock::now() - t0; + EXPECT_TRUE(dt >= std::chrono::milliseconds(150)); + EXPECT_TRUE(dt < std::chrono::milliseconds(1500)); + dragonx::test::g_liteFakeBlockSave = false; // let the abandoned save thread finish + std::this_thread::sleep_for(std::chrono::milliseconds(150)); + } + + // (c) retired -> no save + { + auto c = openSynced(); + c->retire("switch"); + const long before = dragonx::test::g_liteFakeSaveCount.load(); + c->flushOnExit(std::chrono::milliseconds(300)); + EXPECT_EQ(dragonx::test::g_liteFakeSaveCount.load(), before); + } + + // (d) wallet never opened -> no save + { + LiteWalletController c(liteCaps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi())); + const long before = dragonx::test::g_liteFakeSaveCount.load(); + c.flushOnExit(std::chrono::milliseconds(300)); + EXPECT_EQ(dragonx::test::g_liteFakeSaveCount.load(), before); + } +} + // Encryption: the controller drives encrypt/unlock/lock/decrypt + encryptionstatus through the // (injected fake) backend, wipes passphrases, and folds status into WalletState (encrypted/locked). void testLiteWalletControllerEncryption() @@ -18912,6 +18969,7 @@ int main() testLiteRolloutPolicyStagedRollout(); testLiteRolloutManifestLoader(); testLiteWalletControllerRolloutGate(); + testLiteWalletControllerFlushOnExit(); testXmrigReleaseParsing(); testXmrigAssetSelection(); testXmrigChecksumParsing();