fix(rpc): abort any in-flight call before a UI-thread disconnect, not just maintenance calls

Concurrency audit (finding #2, previously held for sign-off): abortMaintenanceCalls() — the choke point run
before every UI-thread rpc_->disconnect() (wallet switch/adopt/restore/rebuild, handleLostConnection,
shutdown) — only called requestAbort() when maint_calls_>0. An ordinary in-flight refresh/scan (not counted
as a maintenance call) holds curl_mutex_ for its whole curl_easy_perform, so disconnect() blocked the UI
for up to the call timeout (≈30s, or 120s for a wallet scan).

Abort unconditionally so disconnect() never waits on the curl lock. The abort flag is cleared by the next
tryConnect()/onConnected() before reconnecting. A user send that happens to be in flight during a teardown
is now aborted too, but that is handled as a lost reply — "outcome unknown, check History", never a
double-send (see the lost-reply classifier, 08a4427) — so no funds are at risk. The z_sendmany/broadcast
call and its args are unchanged.

Threading fix — clean build + ctest; the UI-block is runtime-only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-10-19 18:48:22 -05:00
co-authored by Claude Opus 4.8
parent 4f4c6b7a16
commit d7fee801d3
2 changed files with 9 additions and 5 deletions
+5 -4
View File
@@ -562,10 +562,11 @@ public:
private: private:
App& app_; App& app_;
}; };
// Main thread, right before rpc_->disconnect(): abort rpc_'s in-flight call if it is a maintenance // Main thread, right before rpc_->disconnect(): abort rpc_'s in-flight call (any call, not just a
// call, so the disconnect doesn't freeze the UI for up to its 120 s, and refuse maintenance calls that // maintenance one: disconnect() needs the curl lock the call holds, which would freeze the UI), and
// start before the next connect (onConnected() lifts that). Ordinary calls are left to finish (at most // refuse maintenance calls that start before the next connect (onConnected() lifts that). The abort
// 30 s, as before), so a user's own send is never cut off mid-call. // flag is cleared by the next tryConnect(). A user send aborted mid-flight is handled as a lost reply
// ("outcome unknown — check History"), never a double-send.
void abortMaintenanceCalls(); void abortMaintenanceCalls();
// Whether a whole-wallet scan the user didn't ask for may be sent now: a connected, unlocked, synced // Whether a whole-wallet scan the user didn't ask for may be sent now: a connected, unlocked, synced
// full node that is past warmup, not rescanning and not busy. // full node that is past warmup, not rescanning and not busy.
+4 -1
View File
@@ -1148,7 +1148,10 @@ void App::abortMaintenanceCalls()
{ {
// Both stick until the next connect, which is fine here: every caller disconnects next. // Both stick until the next connect, which is fine here: every caller disconnects next.
maint_abort_.store(true); // refuses maintenance calls still queued on the worker maint_abort_.store(true); // refuses maintenance calls still queued on the worker
if (rpc_ && maint_calls_.load() > 0) rpc_->requestAbort(); // cuts off the one in flight // Unconditional: an ordinary in-flight refresh/scan also holds curl_mutex_ for its whole
// curl_easy_perform, and every caller disconnect()s next. The abort is cleared by the next
// tryConnect() (rpc_) / onConnected() (fast_rpc_) before they reconnect.
if (rpc_) rpc_->requestAbort(); // cuts off whatever call is in flight
} }
bool App::walletScanReady() const bool App::walletScanReady() const