IPC stability bugs

Bug 72

Client can use freed memory if the server closes the socket right away

Status
Fixed
Symptom
Crash; worst effect seen: assertion failure
Where seen
Libmultiprocess tests (mptest)
Trigger
The server side goes away abruptly
First reported
2026-07-10 (first report)
Fixed
2026-08-11
Fix
mp#298
In released binaries
Yes
Defect ever on master
Yes

Summary

A client connecting over IPC could abort or read freed memory if the peer closed the socket immediately: ConnectStream registered the handler that deletes the Connection on disconnect before the ProxyClient that owns the connection existed, so an early EOF could free the Connection while the client constructor was still reading it. It was found by a new test that failed on every NetBSD CI run. Released Bitcoin Core binaries (v30.0 through v31.1) contain the defect, and bitcoin-cli -ipcconnect connecting to a peer that dies at once can reach it. Fixed in mp#298, which reworked error handling when creating clients; not yet in Bitcoin Core's subtree.

What happened

Root cause

ConnectStream (include/mp/proxy-io.h) ran one loop.sync() block on the event-loop thread that created the Connection, took the bootstrap init_client, and registered connection->onDisconnect([..]{ delete connection_ptr; }). Only after that block returned did the caller thread construct ProxyClient<InitInterface>(init_client, connection.release(), destroy_connection=true). Nothing ordered those two steps.

If the socket was already dead, Cap'n Proto hit EOF on its first read and the handler deleted the Connection (whose destructor resets its EventLoopRef m_loop) on the event-loop thread, while the caller thread was entering ProxyContext's constructor (src/mp/proxy.cpp) and reading connection->m_loop from the freed object. The window needs a peer that is gone before the first read and an event-loop thread that is fast relative to the caller, hence the NetBSD-only failure.

The ordering dates from when ConnectStream was written in 2019. The fix's commit message calls it one of "Two bugs that have always been present", which the history confirms.

The code

Quoted from bb473690c97c^. ConnectStream (include/mp/proxy-io.h) creates the Connection and registers its delete-on-disconnect handler on the event-loop thread, then constructs the client that will own it on the calling thread:

loop.sync([&] {
    connection = std::make_unique<Connection>(loop, kj::mv(stream));
    init_client = connection->m_rpc_system->bootstrap(ServerVatId().vat_id).castAs<InitInterface>();
    Connection* connection_ptr = connection.get();
    connection->onDisconnect([&loop, connection_ptr] {
        MP_LOG(loop, Log::Warning) << "IPC client: unexpected network disconnect.";
        delete connection_ptr;
    });
});
return std::make_unique<ProxyClient<InitInterface>>(
    kj::mv(init_client), connection.release(), /* destroy_connection= */ true);

The first thing the client constructor does with the pointer is read from it, in ProxyContext's constructor (src/mp/proxy.cpp):

ProxyContext::ProxyContext(Connection* connection) : connection(connection), loop{*connection->m_loop} {}

If the server end of the socket is already closed:

  1. Event loop: sync() returns after the handler is registered; Cap'n Proto reads EOF and fires onDisconnect.
  2. Event loop: the handler runs delete connection_ptr.
  3. Caller: constructs ProxyClient<InitInterface> with the same pointer, and ProxyContext reads connection->m_loop from freed memory.

The fix drops the handler from ConnectStream and registers it at the end of the ProxyClientBase constructor, once the client owns the connection (bb473690c97c; the construct() cleanup hunk, which fixes a separate leak, and the tests are omitted):

@@ -592,7 +592,28 @@ ProxyClientBase<Interface, Impl>::ProxyClientBase(typename Interface::Client cli
+    // If this client owns the connection, delete the connection on disconnect.
+    if (destroy_connection) {
+        m_context.loop->sync([&] {
+            EventLoop& loop = *m_context.loop;
+            Connection* connection = m_context.connection;
+            connection->onDisconnect([&loop, connection] {
+                MP_LOG(loop, Log::Warning) << "IPC client: unexpected network disconnect.";
+                delete connection;
+            });
+        });
+    }
@@ -840,11 +865,6 @@ std::unique_ptr<ProxyClient<InitInterface>> ConnectStream(EventLoop& loop, Strea
-        Connection* connection_ptr = connection.get();
-        connection->onDisconnect([&loop, connection_ptr] {
-            MP_LOG(loop, Log::Warning) << "IPC client: unexpected network disconnect.";
-            delete connection_ptr;
-        });

Until the client constructor has finished using the Connection, nothing is registered that can delete it; after that, the client's own disconnect cleanup is already in place.

Fix

bb473690c97c removes the handler registration from ConnectStream and makes ProxyClientBase responsible for the connection it owns: when destroy_connection is true, the end of the ProxyClientBase constructor runs a loop->sync() that registers the delete-on-disconnect handler, so the Connection cannot be deleted until the client that owns it is fully set up. The same commit fixes an independent leak when construct() throws (mp#309, bug page); ryanofsky asked for both fixes in one commit and for ProxyClientBase to own the handler.

A consequence noted in the PR description: every client created with destroy_connection=true now deletes its connection on an unexpected disconnect, so a call after a server disconnect can fail with either "called after disconnect" or "interrupted by disconnect" depending on timing. That made a test assertion flaky, fixed in mp#343 (bug page). Moving the handler also exposed a pre-existing thread-map teardown race, addressed in mp#361.

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 cleanup hook armed before its owner exists

  • Prevented by a clear ownership model, a design rule
  • Earliest detector a disconnect test (did not exist then)
  • Diagnosis moderate
  • Fix design moderate
  • Debugging aids a clear assertion message, a hook to force the timing, CI artifacts
  • Rust yes

Would Rust have prevented it? Yes. Capnproto-rust's connection and bootstrap client are Rc and !Send, so a client built on the calling thread could not hold the loop thread's Connection at all (the blocking caller needs a different model); a delete closure could only capture an Rc or Weak, and an Rc keeps the Connection alive until the client drops it too, so the free happens once and late rather than early. More on Rust

Could it have been prevented?

Probably. The fix is the rule: the code that deletes an object on disconnect is registered by the object's owner, after the owner exists. ConnectStream in f324c666150b armed onDisconnect([..]{ delete connection_ptr; }) on the event-loop thread inside the same sync() block that created the Connection, then handed the raw pointer to a ProxyClient constructed later on the calling thread, so for that window the Connection had two deleters (the handler and the future client) and no owner. An ownership model in which the delete-on-disconnect handler is part of ProxyClientBase, which is what bb473690c97c does under destroy_connection, leaves nothing to arm early. The rule was not written down in 2019 and the function's shape (build on one thread, own on another) hid the gap; a raw Connection* captured by a deleting lambda is the visible sign.

Could it have been found sooner?

Yes, by a test that hands ConnectStream a socket whose peer is already gone, which is exactly the test that found it: the "already disconnected" case xyzconstant added in mp#298, seven years after the code was written and eleven months after ryanofsky asked for such tests in mp#183. No disconnect test existed before mp#160 (2025-06), and those disconnected after the client was up. Even with the test, the window is the time between sync() returning and the constructor's first read, so on Linux and macOS runners the caller always won; only the NetBSD job lost every time. ASan or TSan would have made the loss fatal and explicit, but neither runs on a platform slow enough to lose without help.

What made it hard to debug?

Moderately. The CI output was assertion "m_loop" failed in EventLoopRef::operator* with an empty stack, which names the reset member but not the freed object, and the failure could not be reproduced outside NetBSD. xyzconstant asked Claude to reproduce it and the answer was a sleep_for(100ms) between the sync() block and the client construction, which widened the window and made the race fail everywhere; from there the five-step sequence in mp#308 followed in four days. The fix went through one review round (the first version registered the handler in a second sync() after the client existed; ryanofsky asked for ProxyClientBase to own it) and had two follow-on effects, a flaky error-string assertion (mp#343) and exposure of an older thread-map race (mp#361), because every owning client now deletes its connection on disconnect. A permanent test hook that delays the caller between the two steps would keep the interleaving reproducible.

Lesson

Do not arm a callback that deletes an object before the object's owner exists; let the owner register its own cleanup as the last step of its construction, so there is never a moment when something else can free what the owner is about to use.

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
Seen in mptest
Scope production
Trigger Server disconnects abruptly (server-unclean-disconnect)
Phase startup
Side client
Component proxy-io
Root cause kind lifetime
Fix kind local-fix
Detection new-test
Needs a peer that is not libmultiprocess C++ no
Needed a sanitizer to see no
Defect ever on master yes
Released binary
Confidence high