IPC stability bugs

Bug 6

Cancel callback writes to a finished call's stack after a disconnect

Status
Fixed
Symptom
Undefined behavior; worst effect seen: sanitizer report only
Where seen
Libmultiprocess tests (mptest), Bitcoin Core functional tests
Trigger
A client disconnects abruptly, for example because it is killed during a call
First reported
2026-03-10 (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 write to freed stack memory when a client disconnected just as an asynchronous request was finishing. The request cancellation added in mp#240 lets the server stop work on a call whose client has gone away, and its callback could still run after the worker's stack frame it referenced had gone away. AddressSanitizer reported it as a stack-use-after-return in mptest in Bitcoin Core CI four times in two weeks, and Antithesis saw it too; without ASan it is a silent write that can corrupt whatever reuses that stack. Fixed in mp#249 ("fixes for race conditions on disconnects"), before any release contained the bug.

What happened

Root cause

PassField in include/mp/type-context.h registers cancel_monitor.m_on_cancel, a lambda that captures server_context and cancel_mutex by reference. Both are locals of the worker thread's frame that runs the request. The CancelMonitor is owned by ProxyServer<Thread>::post() and was only cleared in the sync() block that runs after fn() returns. If the client disconnected in between, after the frame had been popped but before that clear, the event loop thread destroyed the cancellation probe, which called m_on_cancel, which locked a destroyed mutex and wrote request_canceled into dead stack memory. @ismaelsadeeq drew a sequence diagram of the window, which ryanofsky confirmed.

The window is a few instructions wide. ASan under CI load and Antithesis's schedule exploration both hit it within two weeks of bitcoin#34422, the subtree update that brought mp#240 into Bitcoin Core. The fix's commit message calls it "unlikely to happen normally".

The code

Quoted from 1dbc59a4aa38^. On the worker thread, PassField (include/mp/type-context.h) registers a cancellation callback that captures two of its own locals, cancel_mutex and server_context, by reference:

Mutex cancel_mutex;
Lock cancel_lock{cancel_mutex};
// ...
    cancel_monitor.m_on_cancel = [&server, &server_context, &cancel_mutex, req]() {
        // ...
        Lock cancel_lock{cancel_mutex};
        server_context.request_canceled = true;
    };

The CancelMonitor holding that callback belongs to ProxyServer<Thread>::post() (include/mp/proxy-io.h), which destroys it only in a sync() call made after fn() has returned and those locals are gone:

kj::Maybe<kj::Exception> exception{kj::runCatchingExceptions([&]{ result_value.emplace(fn(*cancel_monitor_ptr)); })};
m_loop->sync([this, &result_value, &exception, self = kj::mv(self), result_fulfiller = kj::mv(result_fulfiller), cancel_monitor_ptr = kj::mv(cancel_monitor_ptr)]() mutable {
    // ...
    cancel_monitor_ptr = nullptr;

Until then, destroying the request's promise still calls the callback, from CancelMonitor::promiseDestroyed (include/mp/util.h):

m_canceled = true;
if (m_on_cancel) m_on_cancel();

If the client disconnects just as the request finishes:

  1. Worker: fn() returns; cancel_mutex and server_context go out of scope.
  2. Event loop: the disconnect destroys the promise; the attached CancelProbe calls promiseDestroyed, which runs m_on_cancel.
  3. Event loop: m_on_cancel locks the destroyed cancel_mutex and writes request_canceled into freed stack memory (the segfault and ASan reports).
  4. Worker: enters the sync() that would have destroyed the monitor, too late.

The fix clears the callback in the sync() block the worker already runs at the end of the invocation, while the locals still exist (1dbc59a4aa38; comment trimmed):

@@ -157,6 +157,15 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn&
                         server.m_context.loop->sync([&] {
+                            cancel_monitor.m_on_cancel = nullptr;
                             auto self_dispose{kj::mv(self)};

Cancellation also runs on the event-loop thread, so once this sync() has cleared m_on_cancel no later cancellation can call it, and one that came earlier ran while the locals were still alive.

Fix

The fix commit 1dbc59a4aa38 sets cancel_monitor.m_on_cancel = nullptr in the sync() block at the end of the request, on the event loop thread, while the frame is still alive, so no later cancellation can reach it (+9, mostly comment). The test "Calling async IPC method, with server disconnect after cleanup" (1643d05ba075, @Sjors) disconnects from a hook placed just before the worker returns.

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 callback outlives what it captured

  • Prevented by a clear ownership model, a review checklist item
  • Earliest detector a disconnect test (did not exist then)
  • Diagnosis easy
  • Fix design easy
  • Debugging aids sanitizer output, a hook to force the timing, symbolized stack traces
  • Rust yes

Would Rust have prevented it? Yes. A closure that captures cancel_mutex and server_context by reference could not be stored in a CancelMonitor that outlives the frame; it would have to own an Arc<Mutex<bool>>, so a late cancel writes into a live allocation nobody reads, which is harmless. More on Rust

Could it have been prevented?

Largely. The m_on_cancel lambda captures two locals of the worker's PassField frame (server_context, cancel_mutex) by reference, but it is stored in a CancelMonitor owned by ProxyServer<Thread>::post(), which cleared it only after fn() had returned and the frame was gone. The ownership rule that would have made this hard to write is that a callback capturing a frame by reference is unregistered by that frame, before it exits, on the thread that can run it; the fix (1dbc59a4aa38) does exactly that by clearing m_on_cancel inside the frame's final loop->sync(). A review checklist line for any reference-capturing lambda, "who owns this closure, and is it guaranteed dead before what it captures?", would have caught it in mp#240: the registration and the clear were in different files and the gap between them was the question.

Could it have been found sooner?

Yes. The test that ships with the fix (1643d05ba075) disconnects from a hook placed just before the worker returns and fails every time without the fix; the hook mechanism from mp#160 existed, only this placement did not. Without it the bug was found the slow way: the existing test "disconnecting and blocking during the call" landed in the few-instruction window under Bitcoin Core's ASan job four times in two weeks (bitcoin#34782), 18 days after the subtree update, and Antithesis saw it independently. The library had no sanitizer CI of its own then, so the first place the interleaving could be seen was the downstream repository.

What made it hard to debug?

It was not, once seen: the ASan stack-use-after-return trace named the frame (PassField's server_context) and the writing thread, ryanofsky posted the diagnosis and patch the next day, and Sjors had the hook test the day after. ismaelsadeeq's sequence diagram of the window is the kind of artifact that would have been worth drawing at design time. Without ASan this is a one-byte write into a reused stack slot, so the non-sanitizer symptom would have been an unrelated crash somewhere else; the sanitizer turned an unattributable corruption into a same-day fix.

Lesson

A closure that captures locals by reference must be unregistered by the frame that owns those locals, before the frame returns, on the thread that can invoke it; storing such a closure in a longer-lived object is a use-after-return waiting for a timing.

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 ub
Worst effect observed sanitizer-report-only
Seen in mptest, functional-test
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 asan
Needs a peer that is not libmultiprocess C++ yes
Needed a sanitizer to see yes
Defect ever on master yes
Released no
Confidence high