From 099d5df3e11f1ba9d24c2bd0f500a7f57529d203 Mon Sep 17 00:00:00 2001 From: DanS Date: Mon, 19 Oct 2026 15:07:18 -0500 Subject: [PATCH] fix(lite): refuse backend calls after a retired bridge is shut down; make the shutdown flag atomic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concurrency audit (lite): LiteClientBridge::shutdownCalled_ was a plain bool, set by retire()/destruction on one thread and read nowhere by the detached sync/console/broadcast threads. A retired controller's in-flight execute() (send/save/sync) therefore still ran litelib_* against the PROCESS-GLOBAL client, which may already belong to a NEW controller after a server switch. - shutdownCalled_ is now std::atomic; shutdown() uses exchange() so exactly one caller tears the client down (idempotent) and the flag is published for the detached threads. - execute() and initializeNew/NewFromPhrase/Existing refuse once the bridge is shut down (shutDownResult), so a stale command can't reach the global client. Gating only — doSend/doShield, the JSON send payload, amounts/memo/recipients, persistAfterBroadcast, encrypt/unlock/export and api_.execute argument passing are byte-identical when the bridge is live. Test: testLiteClientBridgeRefusesAfterShutdown. Co-Authored-By: Claude Opus 4.8 --- src/wallet/lite_client_bridge.cpp | 22 ++++++++++++++++------ src/wallet/lite_client_bridge.h | 7 ++++++- tests/test_phase4.cpp | 21 +++++++++++++++++++++ 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/wallet/lite_client_bridge.cpp b/src/wallet/lite_client_bridge.cpp index f897110..a4b643f 100644 --- a/src/wallet/lite_client_bridge.cpp +++ b/src/wallet/lite_client_bridge.cpp @@ -94,11 +94,11 @@ LiteClientBridge LiteClientBridge::linkedSdxl() LiteClientBridge::LiteClientBridge(LiteClientBridge&& other) noexcept : api_(other.api_), unavailableReason_(std::move(other.unavailableReason_)), - shutdownCalled_(other.shutdownCalled_), + shutdownCalled_(other.shutdownCalled_.load(std::memory_order_acquire)), mutationEpoch_(other.mutationEpoch_.load(std::memory_order_acquire)) { other.api_ = {}; - other.shutdownCalled_ = true; + other.shutdownCalled_.store(true, std::memory_order_release); } LiteClientBridge& LiteClientBridge::operator=(LiteClientBridge&& other) noexcept @@ -107,11 +107,11 @@ LiteClientBridge& LiteClientBridge::operator=(LiteClientBridge&& other) noexcept shutdown(); api_ = other.api_; unavailableReason_ = std::move(other.unavailableReason_); - shutdownCalled_ = other.shutdownCalled_; + shutdownCalled_.store(other.shutdownCalled_.load(std::memory_order_acquire), std::memory_order_release); mutationEpoch_.store(other.mutationEpoch_.load(std::memory_order_acquire), std::memory_order_release); other.api_ = {}; - other.shutdownCalled_ = true; + other.shutdownCalled_.store(true, std::memory_order_release); return *this; } @@ -140,6 +140,7 @@ bool LiteClientBridge::checkServerOnline(const std::string& server) const LiteBridgeStringResult LiteClientBridge::initializeNew(bool dangerous, const std::string& server) { if (!available()) return unavailableResult(); + if (isShutdown()) return shutDownResult(); return takeOwnedString(api_.initializeNew(dangerous, server.c_str())); } @@ -151,6 +152,7 @@ LiteBridgeStringResult LiteClientBridge::initializeNewFromPhrase(bool dangerous, bool overwrite) { if (!available()) return unavailableResult(); + if (isShutdown()) return shutDownResult(); return takeOwnedString(api_.initializeNewFromPhrase( dangerous, server.c_str(), seed.c_str(), birthday, account, overwrite)); } @@ -158,6 +160,7 @@ LiteBridgeStringResult LiteClientBridge::initializeNewFromPhrase(bool dangerous, LiteBridgeStringResult LiteClientBridge::initializeExisting(bool dangerous, const std::string& server) { if (!available()) return unavailableResult(); + if (isShutdown()) return shutDownResult(); return takeOwnedString(api_.initializeExisting(dangerous, server.c_str())); } @@ -180,6 +183,9 @@ LiteBridgeStringResult LiteClientBridge::execute(const std::string& command, con { if (!available()) return unavailableResult(); if (command.empty()) return {false, {}, "lite command is empty"}; + // A retired controller's bridge is shut down; litelib_execute would run on the NEW global client + // (a stale send/save/sync/console command). Refuse it here, once, for every detached thread. + if (isShutdown()) return shutDownResult(); auto result = takeOwnedString(api_.execute(command.c_str(), args.c_str())); if (!commandIsReadOnly(command)) // bump AFTER the command returns, even on failure (be conservative) mutationEpoch_.fetch_add(1, std::memory_order_acq_rel); @@ -188,11 +194,15 @@ LiteBridgeStringResult LiteClientBridge::execute(const std::string& command, con void LiteClientBridge::shutdown() { - if (shutdownCalled_) return; - shutdownCalled_ = true; + if (shutdownCalled_.exchange(true, std::memory_order_acq_rel)) return; // exactly one caller shuts the client down if (available()) api_.shutdown(); } +LiteBridgeStringResult LiteClientBridge::shutDownResult() +{ + return {false, {}, "lite client was shut down"}; +} + LiteBridgeStringResult LiteClientBridge::unavailableResult() const { return {false, {}, unavailableReason_.empty() ? "lite backend is unavailable" : unavailableReason_}; diff --git a/src/wallet/lite_client_bridge.h b/src/wallet/lite_client_bridge.h index a340282..9d6b61c 100644 --- a/src/wallet/lite_client_bridge.h +++ b/src/wallet/lite_client_bridge.h @@ -70,6 +70,10 @@ public: LiteBridgeStringResult initializeExisting(bool dangerous, const std::string& server); LiteBridgeStringResult execute(const std::string& command, const std::string& args); void shutdown(); + // True once shutdown() ran (retire() / destruction / move-from). After this every backend call + // (execute / initialize*) is refused: litelib_* acts on the process-global client, which may by + // then belong to a NEW controller. + bool isShutdown() const { return shutdownCalled_.load(std::memory_order_acquire); } // Monotonic counter bumped AFTER every execute() of a command that could change wallet-visible // data (anything not on the read-only allow-list). The refresh worker samples this to detect @@ -82,12 +86,13 @@ private: LiteClientBridge(LiteClientBridgeApi api, std::string unavailableReason); LiteBridgeStringResult unavailableResult() const; + static LiteBridgeStringResult shutDownResult(); LiteBridgeStringResult takeOwnedString(char* rawValue) const; static bool commandIsReadOnly(const std::string& command); LiteClientBridgeApi api_; std::string unavailableReason_; - bool shutdownCalled_ = false; + std::atomic shutdownCalled_{false}; // read by detached threads in execute() while retire() sets it std::atomic mutationEpoch_{0}; }; diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index 900bab6..a7c2a0e 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -12155,6 +12155,26 @@ void testLiteClientBridgeMutationEpoch() } } +// A retired bridge is shut down; every later backend call must be refused (litelib_* would otherwise run +// on the process-global client, which may belong to a NEW controller by then). shutdown() is idempotent. +void testLiteClientBridgeRefusesAfterShutdown() +{ + using namespace dragonx::wallet; + auto bridge = LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi()); + EXPECT_FALSE(bridge.isShutdown()); + EXPECT_TRUE(bridge.execute("balance", "").ok); + bridge.shutdown(); + bridge.shutdown(); // idempotent (exchange): second call is a no-op + EXPECT_TRUE(bridge.isShutdown()); + const auto before = bridge.mutationEpoch(); + const auto s = bridge.execute("send", "[]"); + EXPECT_FALSE(s.ok); + EXPECT_FALSE(bridge.execute("save", "").ok); + EXPECT_FALSE(bridge.initializeExisting(false, "https://x").ok); + EXPECT_FALSE(bridge.initializeNew(false, "https://x").ok); + EXPECT_TRUE(bridge.mutationEpoch() == before); // a refused send never reached the backend or bumped the epoch +} + // M2b-3 hardening: the backend `sync` is a blocking, uninterruptible full scan. Destroying the // controller while a sync is in flight must NOT hang (the sync thread is detached, not joined). void testLiteWalletControllerShutdownDoesNotHangDuringSync() @@ -19514,6 +19534,7 @@ int main() testLiteWalletControllerWorkerProducesModel(); testLiteWalletControllerWorkerChangeGate(); testLiteClientBridgeMutationEpoch(); + testLiteClientBridgeRefusesAfterShutdown(); testLiteWalletControllerShutdownDoesNotHangDuringSync(); testSerialJobLane(); testLiteGatewayRefreshPolicyHelpers();