fix(rpc): drop a stale connect result after teardown; don't let open() erase a pending abort
Concurrency audit, connect-lifecycle races (no fund-path call touched — connect-result gating + abort-flag lifetime only): - The connect task's MainCb ran onConnected() on any successful probe without re-checking that a wallet switch / adopt / restore / shutdown had already torn the connection down — so a probe that succeeded against the old daemon could start a refresh against one about to be stopped. Capture rpc_session_ when posting (main thread) and, in the MainCb, drop the result (requestAbort + disconnect, not a blocking bare disconnect) when the session changed or daemon_restarting_/shutting_down_ is set. Same guard on the cookie-auth retry post. - RPCClient::open() called clearAbort(), so a requestAbort() issued while a connect was still queued was erased, stalling teardown/shutdown. open() no longer clears it; the two reconnect entry points (tryConnect for rpc_, onConnected for fast_rpc_) clear it on the main thread first, skipped while shutting_down_ so a shutdown abort survives. A fresh client defaults to not-aborted, so first connects are unaffected. HELD (not in this commit): making abortMaintenanceCalls() abort unconditionally (the UI-freeze-on-disconnect finding) — it would also abort an in-flight user send during teardown, a send-lifecycle behaviour change that wants explicit sign-off (it is fund-safe via the lost-reply "outcome unknown" handling, but still a behaviour change). Threading fix — clean build + ctest; the races are runtime-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+29
-5
@@ -563,15 +563,31 @@ void App::tryConnect()
|
||||
attempt, isUsingEmbeddedDaemon() ? "yes" : "no");
|
||||
}
|
||||
|
||||
worker_->post([this, config, daemonStarting, externalDetected, attempt]() -> rpc::RPCWorker::MainCb {
|
||||
// A previous teardown leaves rpc_'s abort flag set (open() no longer clears it, so an abort issued
|
||||
// while this connect is still queued is not erased). This is the reconnect entry point: clear it here
|
||||
// on the main thread, after the daemon_restarting_/shutting_down_ gates above.
|
||||
if (rpc_ && !shutting_down_.load()) rpc_->clearAbort();
|
||||
const std::uint64_t connectSession = rpc_session_; // main thread; a teardown/onConnected bumps it
|
||||
worker_->post([this, config, daemonStarting, externalDetected, attempt, connectSession]() -> rpc::RPCWorker::MainCb {
|
||||
bool connected = rpc_->connect(config.host, config.port, config.rpcuser, config.rpcpassword, config.use_tls);
|
||||
std::string connectErr = rpc_->getLastConnectError();
|
||||
const rpc::RpcFailureKind connectKind = rpc_->getLastConnectFailureKind();
|
||||
bool warmingUp = rpc_->isWarmingUp();
|
||||
std::string warmupStatus = rpc_->getWarmupStatus();
|
||||
|
||||
|
||||
return [this, config, connected, warmingUp, warmupStatus, daemonStarting, externalDetected, attempt, connectErr,
|
||||
connectKind]() {
|
||||
connectKind, connectSession]() {
|
||||
// A probe that succeeded after a wallet switch / adopt / restore / shutdown already tore the
|
||||
// connection down must not run onConnected() against a daemon that is about to be stopped.
|
||||
if (connected && (connectSession != rpc_session_ || daemon_restarting_.load() || shutting_down_.load())) {
|
||||
VERBOSE_LOGF("[connect #%d] Dropping stale connect result (teardown raced the probe)\n", attempt);
|
||||
if (rpc_) {
|
||||
rpc_->requestAbort(); // a worker call may hold curl_mutex_; don't block the UI on disconnect()
|
||||
rpc_->disconnect();
|
||||
}
|
||||
connection_in_progress_ = false;
|
||||
return;
|
||||
}
|
||||
noteDaemonRpcResult(connected, connectKind); // a busy node (timeouts / queue full) feeds the quit guard
|
||||
if (connected) connect_backoff_.reset();
|
||||
if (connected) {
|
||||
@@ -600,12 +616,19 @@ void App::tryConnect()
|
||||
if (rpc::Connection::buildCookieAuthConfig(config, cookieConfig)) {
|
||||
VERBOSE_LOGF("[connect #%d] HTTP 401 — retrying with .cookie auth from %s\n",
|
||||
attempt, cookieConfig.hush_dir.c_str());
|
||||
worker_->post([this, cookieConfig, attempt]() -> rpc::RPCWorker::MainCb {
|
||||
worker_->post([this, cookieConfig, attempt, connectSession]() -> rpc::RPCWorker::MainCb {
|
||||
bool ok = rpc_->connect(cookieConfig.host, cookieConfig.port,
|
||||
cookieConfig.rpcuser, cookieConfig.rpcpassword,
|
||||
cookieConfig.use_tls);
|
||||
return [this, cookieConfig, ok, attempt]() {
|
||||
return [this, cookieConfig, ok, attempt, connectSession]() {
|
||||
connection_in_progress_ = false;
|
||||
if (ok && (connectSession != rpc_session_ || daemon_restarting_.load() || shutting_down_.load())) {
|
||||
if (rpc_) {
|
||||
rpc_->requestAbort();
|
||||
rpc_->disconnect();
|
||||
}
|
||||
return; // stale: a teardown raced the probe
|
||||
}
|
||||
if (ok) {
|
||||
VERBOSE_LOGF("[connect #%d] Connected via .cookie auth\n", attempt);
|
||||
saved_config_ = cookieConfig;
|
||||
@@ -836,6 +859,7 @@ void App::onConnected()
|
||||
fast_worker_->start();
|
||||
}
|
||||
// Connect on the fast worker's own thread (non-blocking to main)
|
||||
if (fast_rpc_ && !shutting_down_.load()) fast_rpc_->clearAbort(); // open() no longer clears a prior teardown's abort
|
||||
fast_worker_->post([this]() -> rpc::RPCWorker::MainCb {
|
||||
bool ok = fast_rpc_->connect(saved_config_.host, saved_config_.port,
|
||||
saved_config_.rpcuser, saved_config_.rpcpassword,
|
||||
|
||||
@@ -216,7 +216,8 @@ bool RPCClient::open(const std::string& host, const std::string& port,
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_HTTPHEADER, impl_->headers);
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_WRITEFUNCTION, WriteCallback);
|
||||
// Progress callback so requestAbort() can unblock an in-flight curl_easy_perform.
|
||||
clearAbort(); // a fresh connection must not start in the aborted state
|
||||
// NOTE: abort is deliberately NOT cleared here: a requestAbort() issued while this connect was queued
|
||||
// must still cut it short. Reconnect callers (App::tryConnect / onConnected) call clearAbort() first.
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_NOPROGRESS, 0L);
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_XFERINFOFUNCTION, xferInfoCallback);
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_XFERINFODATA, this);
|
||||
|
||||
Reference in New Issue
Block a user