IPC stability bugs

Bug 38

Node aborts during shutdown if the wallet process has already exited

Status
Fixed
Symptom
Crash; worst effect seen: crash
Where seen
Branch-build
Trigger
The server side goes away abruptly
First reported
2020-03-05 (first report)
Fixed
2025-08-18
Fix
bitcoin#32345
In released binaries
No; the code was in source releases only
Defect ever on master
Yes

Summary

On the unmerged multiprocess branch where bitcoin-node spawns a bitcoin-wallet process, bitcoin-node aborted during shutdown with an uncaught ipc::Exception (or, in older versions, std::logic_error) if the wallet process had already gone away. Shutdown() made IPC calls to the wallet and had nothing to catch the error a broken connection raises. The unguarded call was on master from 2018 and in releases v22 through v29, but released binaries could not reach it, because their chain clients (the wallet objects the node notifies and stops) run in-process; only the spawned wallet of the unmerged bitcoin#10102 ("Multiprocess bitcoin") branch triggered it. Fixed by bitcoin#32345 ("ipc: Handle unclean shutdowns better").

What happened

Three testers of the bitcoin#10102 branch saw bitcoin-node abort on Ctrl-C:

Root cause

Shutdown() in src/init.cpp calls client->stop() on every node.chain_clients entry. With the wallet in a separate process, each entry is a ProxyClient<ChainClient>, a local object that forwards calls to the wallet process, and stop() is an IPC call to bitcoin-wallet. If the wallet process is already gone, libmultiprocess's clientInvoke (the code that sends a call and waits for the result) reports "IPC client method call interrupted by disconnect." through the event loop's raise(), and Bitcoin Core's IPC log handler (IpcLogFn in src/ipc/capnp/protocol.cpp) throws it as an ipc::Exception. Nothing in Shutdown() caught it, so the node terminated before the rest of shutdown ran. On the bitcoin#10102 branch Shutdown() also calls ChainClient.flush, which is where the 2023 and 2024 aborts were traced (gdb trace); stop() is the only chain-client IPC call Shutdown() makes on master.

The wallet process going away first was usually caused by Ctrl-C reaching it too, which is a separate bug (the spawned process had no SIGINT handling). The fix here makes the node survive a wallet that disappears for any reason, including being killed.

The code

Quoted from 2581258ec200^. Near the end of Shutdown() in src/init.cpp, after flushing the chainstate, the node tells each chain client to stop:

void Shutdown(NodeContext& node)
{
    // ...
    if (node.chainman) {
        LOCK(cs_main);
        for (Chainstate* chainstate : node.chainman->GetAll()) {
            if (chainstate->CanFlushToDisk()) {
                chainstate->ForceFlushStateToDisk();
                chainstate->ResetCoinsViews();
            }
        }
    }
    for (const auto& client : node.chain_clients) {
        client->stop();
    }

When the wallet runs in its own process, client is an IPC proxy and stop() is a remote call to bitcoin-wallet. If that process has already exited (it was killed, or Ctrl-C reached it too), clientInvoke in src/ipc/libmultiprocess/include/mp/proxy-types.h raises "IPC client method call interrupted by disconnect.", which Bitcoin Core's IPC log handler in src/ipc/capnp/protocol.cpp throws as an ipc::Exception. Nothing in Shutdown() catches it, so the node calls std::terminate instead of finishing shutdown.

The fix moves the loop to the start of Shutdown() and catches the exception (2581258ec200):

@@ -298,6 +299,14 @@ void Shutdown(NodeContext& node)
     StopHTTPServer();
+    for (auto& client : node.chain_clients) {
+        try {
+            client->stop();
+        } catch (const ipc::Exception& e) {
+            LogDebug(BCLog::IPC, "Chain client did not disconnect cleanly: %s", e.what());
+            client.reset();
+        }
+    }
     StopMapPort();
@@ -370,9 +379,6 @@ void Shutdown(NodeContext& node)
-    for (const auto& client : node.chain_clients) {
-        client->stop();
-    }

A wallet process that has gone away now costs one debug log line: the proxy for it is destroyed and the node runs the rest of Shutdown() normally.

Fix

Commit 2581258ec200 ("ipc: Handle bitcoin-wallet disconnections") moves the stop() loop to the start of Shutdown(), right after the HTTP server stops, and wraps each call in try/catch (const ipc::Exception&), logging "Chain client did not disconnect cleanly" and resetting the client (+9/-3). Its commit message notes it relies on the disconnect-detection fixes in mp#160 ("refactor: EventLoop locking cleanups + client disconnect exception") to raise the exception promptly. darosior's report also showed a crash inside clientInvoke on a connection that was already gone; that is a separate libmultiprocess bug, fixed in mp#160.

Timeline

Lessons

What would have prevented this bug, found it sooner, or made it easier to debug. The patterns page groups these lessons across all bugs.

Pattern Error paths that skip cleanup or completion: Assuming a remote call cannot fail

  • Prevented by a design rule, a review checklist item
  • Earliest detector a test that kills a process (did not exist then)
  • Diagnosis moderate
  • Fix design easy
  • Debugging aids logging, a reproduction script
  • Rust partly

Would Rust have prevented it? Partly. A remote stop() would return a Result that #[must_use] forces the shutdown code to acknowledge, but the question-mark operator or unwrap reproduces the abort, so the decision to catch and continue is still the author's. More on Rust

Could it have been prevented?

Yes. Shutdown() called client->stop() on each chain client as if it could not fail, which was true in 2018 when every chain client was an in-process wallet. Once a chain client could be a proxy for another process, every call on it became a call that can throw because the peer is gone, and shutdown is the time the peer is most likely gone. The rule is "a call across a process boundary can fail at any time, and code that must complete (shutdown, cleanup) catches and continues"; the fix (2581258ec200) is nine lines of exactly that. It was knowable in 2020: ryanofsky's reply to hebasto already says recovering from sudden disconnects "should be improved later". A review item, "which interface calls does shutdown make, and what happens when each throws", is answerable from init.cpp.

Could it have been found sooner?

Yes, by a functional test that kills the spawned wallet process and then stops the node, which is the reproduction ryanofsky used in mp#123. No such test existed on the branch, and the fix did not add one (the PR's new unit test covers Init and Shutdown in-process). The bug was instead seen three times by people pressing Ctrl-C (2020, 2023, 2024), because Ctrl-C reached the wallet process first, which is the sibling record; that coupling meant each sighting looked like a signal problem rather than a missing catch.

What made it hard to debug?

The three reports were spread over four years and two of them could not be reproduced on demand: Sjors' 2023 mainnet abort got "Connection reset by peer" and ryanofsky could only produce similar errors by killing the wallet mid-call. darosior's 2024 report came with the "IPC client: unexpected network disconnect" log line and a gdb trace, and ryanofsky reproduced it in an hour by comparing SIGTERM with SIGINT. The same report also exposed a null-connection crash inside clientInvoke (the mp#160 record), so the fix had to wait for the library to raise a catchable exception instead of crashing or hanging. Logging the disconnect at the point it was detected is what made the 2024 report actionable where the 2020 one was not.

Lesson

When a local call becomes a remote one, its failure modes change from "cannot" to "whenever the peer dies"; audit the paths that must run to completion, shutdown first, and make them survive a throw from every remote call they make.

Classification

The survey's classification of this bug. The explorer charts these fields across all bugs; the method page explains them.

Field Value
Symptom crash
Worst effect observed crash
Seen in branch-build
Scope production
Trigger Server disconnects abruptly (server-unclean-disconnect)
Phase shutdown
Side client
Component bitcoin-init
Root cause kind unhandled-exception
Fix kind local-fix
Detection manual-test
Needs a peer that is not libmultiprocess C++ no
Needed a sanitizer to see no
Defect ever on master yes
Released source
Confidence high