IPC stability bugs

Bug 37

Double free when a thread exits while its connection is being destroyed

Status
Open, fix under review
Symptom
Crash; worst effect seen: crash
Where seen
Libmultiprocess tests (mptest)
Trigger
The server side goes away abruptly
First reported
2025-07-10 (first report)
Fixed
Not yet
Fix
mp#361
In released binaries
Yes
Defect ever on master
Yes

Summary

When a thread that had made IPC calls exited at the same moment its Connection was destroyed on the event-loop thread, both sides could destroy the same ProxyClient<Thread> entry (the handle to the server thread that runs this thread's calls) in the thread's request_threads / callback_threads maps, giving a heap-use-after-free and a glibc "double free or corruption" abort. This is the thread-exit path of the race family described in mp#189; the in-call path was fixed separately in mp#201 (bug page). It was seen as an intermittent CI crash in a new mptest case. Bitcoin Core v29 through v31.1 contain the code, and the fix commit says it is reachable on master through ConnectStream connections when the peer dies while such a thread exits. A fix is in mp#361, which fixes several disconnect bugs found in review, not yet merged.

What happened

Root cause

Every thread that makes IPC calls owns a thread_local ThreadContext (include/mp/proxy-io.h) with maps of ProxyClient<Thread> keyed by Connection*, guarded on paper by Waiter::m_mutex. Two sides can destroy an entry: the owning thread when it exits (the implicit ~ThreadContext), and the event loop when the connection breaks (the SetThread cleanup callback in src/mp/proxy.cpp, registered with addSyncCleanup). ~ThreadContext took no lock, the SetThread callback used a captured map iterator, and the "who destroys it" decision rested on an unlocked m_disconnect_cb.reset() in the callback, which mp#201's fix had moved outside the mutex.

If the exiting thread's ~ProxyClient<Thread> read that flag in the window, it skipped its loop->sync() handshake and went on; the loop thread's erase then destroyed the same object again, left the ProxyClientBase disconnect callback registered, freed the node, and ran that callback on the freed node. The exiting thread finished its own half-run destructor on the same memory, and glibc caught the corruption at thread exit.

Nothing exercised this until mp#335's test made a call thread exit right after the server disconnected, and mp#298 (merged 2026-08-11) made the client Connection be deleted on the loop thread at that same moment.

Fix

No fix merged yet. 3a4a5eb5f54e in mp#361 (co-authored with Claude) makes removal from the map, under Waiter::m_mutex, the single decision point. An explicit ~ThreadContext swaps both maps out under the lock and destroys them after releasing it, since ~ProxyClient<Thread> blocks in EventLoop::sync() and the loop thread locks Waiter::m_mutex itself. The SetThread callback looks the entry up by Connection* under the lock and returns if it is gone. ~ProxyClient<Thread> checks m_context.connection before removeSyncCleanup. A testing_hook_thread_client_destroy hook lets the new test "Thread exiting while its connection is destroyed" interleave the two sides; it fails on every run without the fix. The fix was first pushed to mp#335 and moved into mp#361 on 2026-09-11.

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 Locks, waits and wakeups: A field locked by some accessors and not others

  • Prevented by a design rule, lock annotations
  • Earliest detector a disconnect test (did not exist then)
  • Diagnosis hard
  • Fix design hard
  • Debugging aids sanitizer output, a hook to force the timing, lock annotations
  • Rust partly

Would Rust have prevented it? Partly. Capnproto-rust capabilities are !Send, so a calling thread could not own proxies that the loop thread erases, ruling out the two-destroyer layout; if the maps were shared through an Arc<Mutex<..>> instead, the unlocked flag read and the double drop would not compile, but the rule to drop entries with the mutex released (the drop blocks on the loop thread) is a deadlock the compiler cannot see. More on Rust

Could it have been prevented?

Probably. The fix states the rule: when two parties may destroy the same entry, removal from the container under the lock is the single decision, and whoever removes it destroys it. Before 3a4a5eb5f54e the decision rested on an unlocked read of m_disconnect_cb, the implicit ~ThreadContext destroyed the maps with no lock at all, and the SetThread callback dereferenced a captured iterator. Waiter::m_mutex guarded the maps "on paper"; -Wthread-safety annotations on request_threads and callback_threads would have flagged the unlocked reset that mp#201 moved outside the lock, though not the implicit destructor, which the analysis does not see. The second destroyer came from 8ba0d03b4439 (mp#106), and the question "the map now has two erasers; which one wins when both run?" was not asked then. It was asked in mp#189 a year later, as Problem 3, and answered for the in-call path only.

Could it have been found sooner?

Yes, by a test in which a thread that made IPC calls exits while its Connection is destroyed on the loop thread, under ASan. It was described in mp#189 (2025-07) but no test exercised it, and mp#201 fixed the in-call path without adding locking to ~ThreadContext. It surfaced by accident 13 months later, when mp#335's drain test made a call thread exit right after a server disconnect and mp#298, merged the day before, made the client Connection be deleted on the loop thread at that moment; push 1 of mp#335 had passed. The fix's testing_hook_thread_client_destroy makes the interleaving deterministic and the test fails on every run without it; the earlier TSan and disconnect jobs had no path that ended a calling thread mid-teardown.

What made it hard to debug?

The CI failure was double free or corruption (out) from __malloc_arena_thread_freeres as a thread exited, a stack that names glibc's per-thread cleanup and nothing else. ryanofsky, working with Claude, reproduced it under gdb and an ASan build, which gave the freed request_threads node, the read in the ProxyClientBase disconnect callback and the freeing erase moments earlier, both inside one ~Connection; from there the interleaving took five steps to write down, including the half-run destructor on the exiting thread. The fix was the largest of the three in mp#361: an explicit ~ThreadContext, lookup by key instead of iterator, a null check on m_context.connection, and the constraint that entries are destroyed with the mutex released because ~ProxyClient<Thread> blocks in EventLoop::sync(). Review found that mp#201's "Synchronization note" stated the lock rule backwards (enirox001), corrected in 019b699bc9e7. Clang lock annotations on the maps, and an ASan job on the disconnect tests, would have shortened the path.

Lesson

A lock that one of two destroyers does not take protects nothing; give every shared entry a single removal point under the lock, let removal decide who destroys, and write down which locks may be held while blocking on another thread, then test the rule with a hook that forces both sides into the window.

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 mptest
Scope production
Trigger Server disconnects abruptly (server-unclean-disconnect)
Phase any
Side client
Component thread-map
Root cause kind missing-lock
Fix kind local-fix
Detection review
Needs a peer that is not libmultiprocess C++ no
Needed a sanitizer to see no
Defect ever on master yes
Released binary
Confidence high