Bug 44

Node aborts at shutdown while an IPC mining call is still running

Status
Open, fix under review
Symptom
Assertion failure; worst effect seen: assertion failure
Where seen
A production node, Bitcoin Core functional tests
Trigger
The process closes its own connection while the peer is still there
First reported
2025-09-14 (first report)
Fixed
Not yet
Fix
bitcoin#35932, mp#335
In released binaries
Yes (v30.0-v31.1)
Defect ever on master
Yes

Summary

bitcoin-node can abort on Assert(m_node.chainman) when it shuts down while an IPC mining client has a call in progress. During shutdown the node disconnects its IPC clients and then frees the ChainstateManager. Disconnecting does not stop a server method that is already running on a libmultiprocess worker thread, so that method can call MinerImpl::chainman() after the pointer has been cleared. It was seen with a Stratum v2 client (sv2-tp) when the node was stopped with Ctrl-C, and in an Antithesis run. Released binaries v30.0 through v31.1 have the bug. The fix is in two open PRs, bitcoin#35932 and mp#335.

What happened

Root cause

Shutdown() (src/init.cpp) calls ipc->disconnectIncoming() and then node.chainman.reset(). CapnpProtocol::disconnectIncoming (src/ipc/capnp/protocol.cpp) only removed the incoming mp::Connection objects on the event loop thread. Destroying a connection cancels the KJ promise of every in-flight request ("Interrupted by disconnect"). But a server method that ProxyServer<Thread>::post() has already handed to a worker thread is not interrupted; it runs to completion on that thread. So disconnectIncoming() returned, the shutdown thread cleared m_node.chainman, and a still-running MinerImpl method (in the Antithesis log, checkBlock) called chainman(), which is *Assert(m_node.chainman).

The fix ryanofsky sketched in bitcoin#33387 was to have shutdown wait for a barrier queued on the async cleanup thread with addAsyncCleanup. That covers requests that had not started: their ProxyServer objects are destroyed on disconnect, and their cleanup is queued ahead of the barrier. It does not cover a request that is executing, because Cap'n Proto keeps that request's ProxyServer objects alive until the method returns, so their cleanup is queued after the barrier. The Antithesis log ("canceled while executing") is that second case.

The race needs a call to be in flight when shutdown starts. A C++ client making occasional synchronous calls rarely hits it, but sv2-tp and the Antithesis workload make mining calls continuously from another process.

Fix

No fix is merged yet. The proposed fix, by ryanofsky with Claude, has two parts:

The fix leaves three documented gaps. Destructors of the server objects (m_impl) that run on the async cleanup thread are not waited for. The parent connection, which stays open, is not drained. New connections can still be accepted during shutdown; mp#269, which adds a connection limit to ListenConnections, proposes a listener API for that. Calls waiting in waitTipChanged() or waitNext() need no wait, because Interrupt() wakes them before Shutdown() runs.

Next steps

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 Serving before ready, freeing before done: Cancelling work without waiting for it to stop

  • Prevented by a design rule, a different API shape
  • Earliest detector a disconnect test (did not exist then)
  • Diagnosis moderate
  • Fix design hard
  • Debugging aids logging, thread names
  • Rust partly

Would Rust have prevented it? Partly. A worker task could not hold a plain &ChainstateManager past Shutdown, so it would hold an Arc that keeps chainman alive through the in-flight call, turning the abort into a body running on state the node considers shut down; and in capnproto-rust a server method is a future on the loop thread that disconnect drops, so only a body handed to a worker for blocking work can outlive its connection, and that hand-off has the same cancel-versus-join gap waitDrained() closes. More on Rust

Could it have been prevented?

Probably, by a rule the fix now embodies: a shutdown step that stops accepting work must also wait for work already running before the next step frees what that work uses. bitcoin#32345 deliberately stopped waiting for clients to disconnect (to fix a hang) and inserted disconnectIncoming() before chainman.reset(), treating "connection destroyed, promises cancelled" as "no server code running". It was not: a method body already posted to a worker thread runs to completion, and cancellation in libmultiprocess never promised otherwise. The API shape that would have made this hard to write is one where the disconnect call itself returns only after in-flight bodies have released their server objects, which is what waitDrained() adds; without it, every caller of disconnect has to know the difference between cancel and join. The trap was knowable in 2025, since the same distinction had already come up for ProxyServer lifetimes (mp#160), but the fix that introduced it was itself a response to a shutdown hang, and the two failure modes pull in opposite directions.

Could it have been found sooner?

Yes, by a test that starts a slow server call, disconnects from the server side, and then frees the state the call uses; that is the "draining in-flight server call after disconnect" case mp#335 adds. Nothing like it existed when bitcoin#32345 merged: the library's disconnect tests (from 2025-06) disconnected from the client side, and no functional test stopped the node with an IPC call in flight. TheCharlatan hit it 27 days after the merge with sv2-tp, which keeps a mining call in flight almost always, so any test that shut the node down under a continuously calling client would have found the first form quickly. The second form, an executing request whose server objects Cap'n Proto keeps alive past the barrier, was only shown by Antithesis a year later, and it is the form a barrier-based test would have passed.

What made it hard to debug?

The first report was closed by its reporter as a version mismatch because it did not reproduce; ryanofsky reopened it from reasoning about the shutdown order, not from a trace, and sketched a barrier fix. That sketch was found insufficient only when the Antithesis log in bitcoin#35845 showed request #46 canceled while executing followed by the assert, which distinguished the executing case from the queued one; the log lines added by mp#240 and the b-capnp-loop thread name made that reading possible. The fix then took the largest design in the ledger (288 lines, a tracker, a disconnect() that keeps the object, a wait that must run off the loop thread to avoid deadlock, three documented gaps) and is still open. What would have helped is a count of live server objects per connection that the node could log at shutdown, which is what the fix's pendingServerObjects() provides.

Lesson

Cancelling is not joining: after telling peers to stop, wait for the work already running before freeing what it uses, and make the stop API return only when that wait is done, so callers cannot get the order wrong.

Notes

Classification

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

Field Value
Symptom assert
Worst effect observed assert
Seen in production-node, functional-test
Scope production
Trigger Process closes its own connection (local-disconnect)
Phase shutdown
Side server
Component bitcoin-ipc
Root cause kind ordering
Fix kind new-code-path
Detection user-report
Needs a peer that is not libmultiprocess C++ no
Needed a sanitizer to see no
Defect ever on master yes
Released binary
Confidence high