IPC stability bugs

Bug 3

Node crashes when a disconnected client's call returns a new object

Status
Fixed
Symptom
Crash; worst effect seen: crash
Where seen
A production node, Bitcoin Core functional tests
Trigger
A client disconnects abruptly, for example because it is killed during a call
First reported
2025-10-06 (first report)
Fixed
2026-02-20
Fix
mp#240
Shipped to Bitcoin Core
bitcoin#34422
In released binaries
Yes (v30.0-v30.3)
Defect ever on master
Yes

Summary

bitcoin-node crashed when an IPC client disconnected while one of its calls was still running on the server and that call then returned an interface pointer, such as a BlockTemplate from waitNext. Nothing cancelled a server request when its connection went away. So when the call finished, the worker thread built a ProxyServer for the returned object from the already-deleted Connection, aborting on Assertion 'm_loop' failed or segfaulting. Rust Stratum v2 clients hit it repeatedly against v30.0rc3 and v30.2. Fixed in mp#240 ("Avoid errors from asynchronous (non-c++) clients"), which adds request cancellation, and shipped in Bitcoin Core v31; v30.0 through v30.3 have the bug.

What happened

Root cause

When a connection died, Connection::~Connection tore down the RPC system and the ProxyServer objects, but a worker thread already running a request kept executing the C++ method. When the method returned a std::unique_ptr<BlockTemplate>, MakeProxyServer in include/mp/type-interface.h built a ProxyServer<BlockTemplate> from InvokeContext::connection, a reference to the deleted Connection, and used its dangling EventLoopRef. In the bitcoin#35545 variant the request had not started: its queued callback in PassField (include/mp/type-context.h) had captured the connection, and SetThread dereferenced connection->m_loop. Both are the same defect: nothing cancels a request whose connection has been destroyed.

A C++ client does not easily trigger this, because its calls block and its proxies are destroyed before it disconnects. Rust and Python clients can drop a connection with a long waitNext outstanding, and the Stratum v2 pool did exactly that on shutdown.

The code

Quoted from 0174450ca2e9^. The worker thread runs the server method and then builds the results. ServerRet::invoke (include/mp/proxy-types.h):

auto&& result = Parent::invoke(server_context, TypeList<>(), std::forward<Args>(args)...);
auto&& results = server_context.call_context.getResults();
InvokeContext& invoke_context = server_context;
BuildField(TypeList<decltype(result)>(), invoke_context, Make<StructField, Accessor>(results),
    std::forward<decltype(result)>(result));

invoke_context.connection is a Connection& bound when the request started executing. When the result is an interface pointer, such as the std::unique_ptr<BlockTemplate> from waitNext, MakeProxyServer (include/mp/type-interface.h) wraps it in a new server object for that connection, and the ProxyContext constructor (src/mp/proxy.cpp) follows the connection's event-loop reference:

    return kj::heap<ProxyServer<Interface>>(std::move(impl), context.connection);
// ...
ProxyContext::ProxyContext(Connection* connection) : connection(connection), loop{*connection->m_loop} {}

Nothing tells the worker that the connection has gone away:

  1. Worker: runs waitNext, which blocks until fees rise.
  2. Event loop: the client disconnects without waiting for the reply, and ~Connection destroys the connection. The request is not cancelled.
  3. Worker: waitNext returns a new template; MakeProxyServer passes the freed Connection to ProxyContext, which reads m_loop from it and crashes.

In the bitcoin#35545 variant the request had not started yet when its client disconnected. The worker's first step, SetThread(..., server.m_context.connection, ...) inside PassField (include/mp/type-context.h), then used the destroyed connection's m_loop and hit the Assertion failed: (m_loop) abort.

The fix, 0174450ca2e9 in mp#240, is 251 lines across seven files, so no diff is shown. It adds a Connection::m_canceler that wraps each request's promise and is cancelled from ~Connection. A probe attached in ProxyServer<Thread>::post() records the cancellation, and PassField turns it into a request_canceled flag, checked on the event-loop thread before the request runs; ServerCall::invoke checks it after the method returns and throws instead of building results, and PassField catches and logs that exception. A cancel_mutex keeps the event loop from freeing the params and results while the worker is using them.

Once ~Connection has run, every request of that connection sees request_canceled set, before it starts or before it builds results, so the worker never passes the destroyed Connection to SetThread or MakeProxyServer.

Fix

The fix commit 0174450ca2e9 adds a cancellation path (+251/-16, no tests):

The commit message cites bitcoin#34250 and describes the in-execution case only. ryanofsky explained that the "canceled before it executes" branch covers the bitcoin#35545 variant, which had looked theoretical. Tests came in Bitcoin Core: the "disconnect during BlockTemplate.waitNext" case in interface_ipc.py, first proposed in bitcoin#34284 ("Add tests for unclean disconnect and thread busy behavior") and folded into bitcoin#34422.

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 An object outliving what it points to: A child object outlives its parent

  • Prevented by a design rule, a clear ownership model
  • Earliest detector a disconnect test (did not exist then)
  • Diagnosis moderate
  • Fix design hard
  • Debugging aids a clear assertion message, a reproduction script, logging
  • Rust yes

Would Rust have prevented it? Yes. The Connection reference the worker keeps across a long method could not outlive the connection: it would be an Arc, delaying teardown until waitNext returns (possibly the client's whole timeout), or a Weak whose None the worker must handle; in capnproto-rust a server method is a future the RPC layer drops on disconnect, so cancellation comes with the model, though blocking work moved to another thread still needs the hand-off the fix designs. More on Rust

Could it have been prevented?

Partly. The initial commit (06e1045baba0) handed each request to a worker thread with a bare Connection& and gave ~Connection no way to reach the worker; the request's lifetime was nested inside the connection's only by assumption. A rule the fix now embodies, "any work a connection posts to another thread is cancelled or awaited by the connection's destructor, and the worker rechecks before touching the connection again", would have made the omission visible in 2019, since Cap'n Proto's promise cancellation was already the mechanism the fix uses. It would not have made the fix small: the honest reading of 0174450ca2e9 (seven files, +251/-16, a cancel_mutex added only after TSan objected in bitcoin#34422) is that cancellation on a threaded server is a mechanism, not a check, and the three regressions it produced within weeks (mp#245, mp#249) show the mechanism was not obvious even with the rule in hand.

Could it have been found sooner?

Yes, by a test that kills a client while a server call that returns an interface pointer is blocked, then lets the call return. No such test existed in either project until the fix: the library's disconnect tests from mp#160 (2025-06) covered disconnect during a call but not a call returning a new object afterwards, and the C++ client's blocking calls and orderly destroy meant nothing in-tree abandoned a request. The first peer that did was a Rust Stratum v2 pool being Ctrl-C'd during waitNext, in the v30.0 release candidates. The "disconnect during BlockTemplate.waitNext" case in interface_ipc.py (bitcoin#34284, folded into bitcoin#34422) is that test; the library commit itself carries none.

What made it hard to debug?

Recognizing the site was quick once the steps were isolated: plebhash's recipe (Ctrl-C during waitNext, wait for a fee change) let ryanofsky name MakeProxyServer and the deleted Connection in mp#226 two days after the first report. What was hard was everything around that: the first trace was mixed with a different abort in the same issue, the later reports (bitcoin#34250, bitcoin#34664, bitcoin#35545) each arrived as a new crash and were matched to the same defect weeks or months later, and the fix took four months to start because it needed a design rather than a patch. The assert(m_loop) message pointed at a dead EventLoopRef but not at whose call was still running; the "IPC server destroy" and, after the fix, "request #N interrupted (canceled)" log lines are what let a reader pair a disconnect with the call it orphaned. A log of requests still executing per connection at ~Connection would have made every later sighting recognizable from its log alone.

Lesson

Work dispatched to another thread on behalf of a connection outlives that connection unless something cancels or joins it; build the cancel path when the dispatch is written, and have the worker recheck its owner is alive at every point it reaches back.

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 crash
Worst effect observed assert, crash
Seen in production-node, functional-test
Scope production
Trigger Client disconnects abruptly (client-unclean-disconnect)
Phase steady-state
Side server
Component type-context
Root cause kind missing-cancellation
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