IPC stability bugs

Bug 7

Server crashes when a client drops a new worker thread right away

Status
Fixed
Symptom
Crash; worst effect seen: crash
Where seen
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-02 (first report)
Fixed
2026-03-27
Fix
mp#249
Shipped to Bitcoin Core
bitcoin#34804, bitcoin#34952
In released binaries
Yes (v30.0-v30.3)
Defect ever on master
Yes

Summary

An IPC server could crash with a null-pointer dereference when a client created a server thread with ThreadMap.makeThread and dropped it immediately: the new worker thread's Waiter could be destroyed before the thread had locked it. Bitcoin Core's UBSan CI job hit it in interface_ipc.py, and Antithesis hit it as a segfault. The race has existed since makeThread was written. Fixed in mp#249 ("fixes for race conditions on disconnects") by taking the lock earlier; v30.0 through v30.3 binaries have the bug.

What happened

Root cause

ProxyServer<ThreadMap>::makeThread in src/mp/proxy.cpp starts a worker thread that creates g_thread_context.waiter, calls thread_context.set_value() to hand its context to the event loop, and only then runs Lock lock(g_thread_context.waiter->m_mutex) and waits for a shutdown signal. Once set_value() has run, the event loop can create the ProxyServer<Thread> and return it to the client. If the client drops it at once, ~ProxyServer<Thread> sets waiter to null (the shutdown signal), and if that happens before the worker reaches the Lock line, the worker dereferences null. @ismaelsadeeq drew a timeline of the race, which ryanofsky confirmed.

The window is between two adjacent lines and needs a thread to be created and released immediately. bitcoin#34568 ("mining: Break compatibility with existing IPC mining clients", merged 2026-02-20) added run_deprecated_mining_test to interface_ipc.py. Its Python client created a thread context it never used and then closed the connection, which is exactly that pattern.

The code

Quoted from f11ec29ed208^ (src/mp/proxy.cpp). ProxyServer<ThreadMap>::makeThread starts the worker thread, which publishes its ThreadContext before it locks the waiter's mutex:

std::thread thread([&loop, &thread_context, from]() {
    g_thread_context.thread_name = ThreadName(loop.m_exe_name) + " (from " + from + ")";
    g_thread_context.waiter = std::make_unique<Waiter>();
    thread_context.set_value(&g_thread_context);
    if (loop.testing_hook_makethread_created) loop.testing_hook_makethread_created();
    Lock lock(g_thread_context.waiter->m_mutex);
    // Wait for shutdown signal from ProxyServer<Thread> destructor (signal
    // is just waiter getting set to null.)
    g_thread_context.waiter->wait(lock, [] { return !g_thread_context.waiter; });
});
auto thread_server = kj::heap<ProxyServer<Thread>>(m_connection, *thread_context.get_future().get(), std::move(thread));

The shutdown signal in ~ProxyServer<Thread>, on the event-loop thread, is taking the waiter out of that same context:

const Lock lock(m_thread_context.waiter->m_mutex);
//! Reset thread context waiter pointer, as shutdown signal for done
//! lambda passed as waiter->wait() argument in makeThread code below.
waiter = std::move(m_thread_context.waiter);

If the client releases the new thread right away:

  1. Worker: set_value() hands its context to the event loop.
  2. Event loop: builds the ProxyServer<Thread>; the client drops it, so ~ProxyServer<Thread> locks the mutex, moves waiter out (leaving g_thread_context.waiter null) and waits in join().
  3. Worker: reaches Lock lock(g_thread_context.waiter->m_mutex) and dereferences the null pointer.

The fix takes the lock before publishing the context (f11ec29ed208):

@@ -418,9 +418,9 @@ kj::Promise<void> ProxyServer<ThreadMap>::makeThread(MakeThreadContext context)
         g_thread_context.waiter = std::make_unique<Waiter>();
+        Lock lock(g_thread_context.waiter->m_mutex);
         thread_context.set_value(&g_thread_context);
         if (loop.testing_hook_makethread_created) loop.testing_hook_makethread_created();
-        Lock lock(g_thread_context.waiter->m_mutex);

The destructor needs the same mutex to take the waiter, and the worker releases it only inside wait(), so the waiter cannot disappear before the worker is waiting on it.

Fix

The fix commit f11ec29ed208 moves the Lock line above thread_context.set_value(), so ~ProxyServer<Thread>, which must take the same mutex to signal shutdown, waits until the worker is inside its wait loop (+1/-1). The test "Worker thread destroyed before it is initialized" (a1d643348f4e, @Sjors) disconnects from a makeThread hook and delays the worker to reproduce it.

Before the library fix, Bitcoin Core avoided the trigger in its test: bitcoin#34715 ("test: avoid interface_ipc.py race and null pointer dereference", merged 2026-03-04) stopped run_deprecated_mining_test from creating the unused thread. @l0rinc reproduced the race by adding a usleep(250000) between set_value() and the Lock. bitcoin#34711 was closed against that test change, three weeks before the library fix merged.

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: Serving calls before startup finishes

  • Prevented by a design rule, 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 partly

Would Rust have prevented it? Partly. The waiter would be an Option behind a Mutex or an Arc, so the destructor's take() either panics the worker on unwrap() or leaves it holding a live Waiter nobody will signal; the missing hand-off ordering (lock before publishing the context) is a protocol bug Rust does not enforce. More on Rust

Could it have been prevented?

Yes, by a rule about hand-offs between threads: a thread that publishes a handle to itself (thread_context.set_value()) must already hold whatever the receiving side will need to shut it down, so that from the receiver's first instruction the thread is in the state its destructor expects. The initial commit (06e1045baba0) published first and locked second, leaving a two-line window in which ~ProxyServer<Thread> could take the mutex, null the Waiter and join, while the worker had yet to dereference it. The fix (f11ec29ed208) swaps the two lines, so the destructor cannot get the mutex until the worker is inside wait(). In review the question "after set_value, what can the other side do to this thread, and is the thread ready for it?" is answerable from the two functions side by side; the ismaelsadeeq timeline drawn during the fix review is what such a review would have produced in 2019.

Could it have been found sooner?

Yes, by a test that makes a thread and drops it before using it, with a hook that stalls the worker between publish and lock; that is the "Worker thread destroyed before it is initialized" test (a1d643348f4e) shipped with the fix, and l0rinc showed a usleep in the same spot makes the race deterministic. Nothing in-tree ever created a Thread without using it: the C++ client makes a thread only to call on it. The first peer to do so was the Python client in run_deprecated_mining_test (bitcoin#34568, 2026-02-20), and UBSan on the functional test reported the null dereference within a week (bitcoin#34711), so for seven years the missing piece was the workload, not the tool.

What made it hard to debug?

It was not. The UBSan message named proxy.cpp:416 and the type (mp::Waiter), and ryanofsky diagnosed it with a patch the same day; the Antithesis SEGV in __pthread_mutex_lock from makeThread (bitcoin#34756) was matched to it on sight. The only friction was organizational: Bitcoin Core closed its issue against a test-side workaround (bitcoin#34715) three weeks before the library fix merged, so a reader of the Core tracker would have thought the bug gone while v30 binaries still had it.

Lesson

Before publishing a handle to another thread or party, acquire what that party will need in order to tear you down; the order of "make reachable" and "become ready" is a race whenever it is the wrong way round.

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 functional-test, mptest
Scope production
Trigger Client disconnects abruptly (client-unclean-disconnect)
Phase steady-state
Side server
Component thread-map
Root cause kind ordering
Fix kind one-line-tweak
Detection ci-intermittent
Needs a peer that is not libmultiprocess C++ yes
Needed a sanitizer to see no
Defect ever on master yes
Released binary
Confidence high