IPC stability bugs

Bug 5

Server reads freed request parameters when a client disconnects right after calling

Status
Fixed
Symptom
Crash; worst effect seen: crash
Where seen
A production node, Bitcoin Core functional tests, libmultiprocess tests (mptest)
Trigger
A client disconnects abruptly, for example because it is killed during a call
First reported
2026-03-09 (first report)
Fixed
2026-03-27
Fix
mp#249
Shipped to Bitcoin Core
bitcoin#34804, bitcoin#34952
In released binaries
No
Defect ever on master
Yes

Summary

An IPC server could crash with a use-after-free when a client disconnected right after sending an asynchronous request: the worker thread read the request's parameters after the disconnect had cancelled the request and Cap'n Proto had freed them. It was seen in Antithesis runs, in libmultiprocess sanitizer CI, and as a plain segfault with a Rust mining client against a Bitcoin Core master build. It was introduced by the request cancellation added in mp#240, which lets the server stop work on a call whose client has gone away. Fixed in mp#249 ("fixes for race conditions on disconnects") and shipped before any release contained the bug.

What happened

Root cause

The async-request lambda in PassField (include/mp/type-context.h) began with const auto& params = call_context.getParams(); before checking whether the request had been cancelled. Since 0174450ca2e9, the mp#240 commit that added cancellation, a disconnect cancels the request promise through Connection::m_canceler, and Cap'n Proto frees the request's params when the promise is cancelled. If the cancel lands after the worker thread is scheduled but before it runs, the worker reads freed memory. The cancellation check that follows (cancel_monitor.m_canceled, run under loop->sync) would have returned early, but by then the freed params had already been read.

Before mp#240 requests were never cancelled, so calling getParams() at any time was safe. The cancel has to land between the worker being scheduled and its first getParams() call, which takes a client that disconnects right after sending a request; a blocking C++ client could only do this by being killed in that window.

The code

PassField in include/mp/type-context.h, as of f5509a31fcca^. invoke runs on the worker thread; the sync() lambda and the code at the bottom run on the event-loop thread:

auto invoke = [self = kj::mv(self), call_context = kj::mv(server_context.call_context), &server, req, fn, args...](CancelMonitor& cancel_monitor) mutable {
            // ...
            const auto& params = call_context.getParams();
            Context::Reader context_arg = Accessor::get(params);
            // ...
                server.m_context.loop->sync([&] {
                    // Detect request being canceled before it executes.
                    if (cancel_monitor.m_canceled) {
                        server_context.request_canceled = true;
                        return;
                    }
                    // ...
                    std::tie(request_thread, inserted) = SetThread(
                        GuardedRef{thread_context.waiter->m_mutex, request_threads}, server.m_context.connection,
                        [&] { return context_arg.getCallbackThread(); });
                });
// ...
return server.m_context.connection->m_canceler.wrap(kj::mv(result));

The worker calls call_context.getParams() first and checks cancel_monitor.m_canceled only afterwards. If the client disconnects after the request is posted to the worker but before the worker runs:

  1. Event loop: invoke is posted to the worker thread named in the request.
  2. Event loop: the client disconnects, m_canceler cancels the wrapped promise, and Cap'n Proto frees the call context and its params.
  3. Worker: call_context.getParams() makes a virtual call on the freed CallContextHook (the ASan, TSan and UBSan reports all point at this line).
  4. Worker: the m_canceled check would have returned early, but too late.

The fix drops the early read and reads the params inside the sync() lambda, after the cancellation check (f5509a31fcca, comment change trimmed):

@@ -76,8 +76,6 @@
                 if (loop.testing_hook_async_request_start) loop.testing_hook_async_request_start();
-                const auto& params = call_context.getParams();
-                Context::Reader context_arg = Accessor::get(params);
                 ServerContext server_context{server, call_context, req};
@@ -128,10 +126,13 @@
                         std::tie(request_thread, inserted) = SetThread(
                             GuardedRef{thread_context.waiter->m_mutex, request_threads}, server.m_context.connection,
-                            [&] { return context_arg.getCallbackThread(); });
+                            [&] { return Accessor::get(call_context.getParams()).getCallbackThread(); });
                     });

Cancellation also runs on the event-loop thread, so once the sync() lambda has seen m_canceled false, the params cannot be freed before getParams() returns.

Fix

The fix commit f5509a31fcca removes the early getParams() and reads Accessor::get(call_context.getParams()).getCallbackThread() inside the sync block, after the not-cancelled check, on the event loop thread where cancellation cannot interleave (+5/-4). The follow-up refactor ff1d8ba172ad narrows the outer params scope for the same reason. The test "Calling async IPC method, with server disconnect racing the call" (4a60c39f24a5, @Sjors) disconnects from a hook at request start and crashes without the fix.

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: Reading a request after it was cancelled

  • Prevented by a different API shape, a review checklist item
  • Earliest detector a disconnect test (did not exist then)
  • Diagnosis easy
  • Debugging aids other, a hook to force the timing
  • Rust yes

Would Rust have prevented it? Yes. A capnproto-rust params reader borrows the call context, so it could not be handed to a worker thread or read after the call was cancelled and its context dropped; the worker would have to be given an owned copy of what it needs before the hand-off, which is what reading getCallbackThread() inside the loop-thread sync amounts to. More on Rust

Could it have been prevented?

Largely. mp#240 added a new lifetime rule, "once the request promise is cancelled its params are freed", and guarded the uses it knew about with cancel_mutex, but the worker lambda's first line already read call_context.getParams() before any check. The fix moves the read inside the loop->sync() block after the m_canceled test; the rule it implies is that a worker must not touch a cancellable request outside the section that has proved it uncancelled. An API shape that enforces that (the worker gets the params only from the guarded section, not by capturing call_context) would have made the bad line unwritable. Failing that, a review checklist item for any change that adds cancellation or early destruction, "list every access to the object and show each one is ordered after the check", would have found this line: it is the first line of the lambda the commit was editing.

Could it have been found sooner?

Yes. The test that ships with the fix (4a60c39f24a5) disconnects from a hook at request start and crashes every time without the fix; only the hook was missing, and mp#160 had already shown how to disconnect from inside a server call. Instead the bug was caught 17 days after merge by Antithesis under ASan (bitcoin#34777), once the subtree reached Bitcoin Core, and twice more by the sanitizer jobs of the new libmultiprocess CI. A plain (non-sanitizer) run needed the Rust client to hit it one run in three, since the freed memory usually still holds readable data.

What made it hard to debug?

It was not. The ASan trace pointed at type-context.h:77, the diagnosis and patch followed the same morning (issuecomment-4024285314, four hours after the report), and Sjors had a hook-driven test the next day. Two things were less easy: the UBSan "invalid vptr" sighting on mp#256 was only attributed to this bug by the survey, and the Rust-client crash arrived the day after the fix merged, so the report-to-attribution loop depended on ryanofsky recognizing the trace. Cancellation-aware logging ("request #N interrupted (canceled)") made that recognition possible.

Lesson

When you add cancellation or early destruction to an existing path, the new rule applies to every access that already exists, not only the ones you add; enumerate them, and give the test a hook that forces the cancel into the widest window rather than relying on a sanitizer to see it by chance.

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 crash
Seen in production-node, functional-test, mptest
Scope production
Trigger Client disconnects abruptly (client-unclean-disconnect)
Phase steady-state
Side server
Component type-context
Root cause kind lifetime
Fix kind local-fix
Detection antithesis
Needs a peer that is not libmultiprocess C++ yes
Needed a sanitizer to see no
Defect ever on master yes
Released no
Confidence high