DragonX/ObsidianDragon
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 10 Wiki Activity

fix(lite): refuse backend calls after a retired bridge is shut down; make the shutdown flag atomic

Browse Source 
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<bool>; 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 <noreply@anthropic.com>
This commit is contained in:
DanS
2026-10-19 15:07:18 -05:00
co-authored by Claude Opus 4.8
parent df98f98669
commit 099d5df3e1
3 changed files with 43 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/wallet/lite_client_bridge.cpp
+16 -6
View File
@@ -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_};
src/wallet/lite_client_bridge.h
+6 -1
View File
@@ -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<bool> shutdownCalled_{false}; // read by detached threads in execute() while retire() sets it
std::atomic<std::uint64_t> mutationEpoch_{0};
};
tests/test_phase4.cpp
+21
View File
@@ -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();