IPC stability bugs

Bug 73

Event loop never exits if a client's setup call fails while it is created

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

Summary

Some interfaces, such as Bitcoin Core's Init interface, declare a construct() method that the client calls while its ProxyClient is being created. If that call threw, the client constructor exited by exception, its destructor never ran, and the Connection and the EventLoopRef it holds were leaked, so EventLoop::loop() never returned and any thread joining the event loop hung. It was found when a new mptest case hung the whole suite. Released Bitcoin Core binaries (v30.0 through v31.1) contain the code, but there the leak needs construct() to throw on a live connection, which Bitcoin Core's Init.construct does not do short of running out of memory; no user-facing hang is known. Fixed in mp#298, which reworked error handling when creating clients; not yet in Bitcoin Core's subtree.

What happened

Root cause

When the init interface declares construct(), the generated ProxyClient constructor (ProxyClientBase::ProxyClientBase in include/mp/proxy-io.h) calls Sub::construct(*this), a blocking IPC call, as its last step. If that call throws (here with "IPC client method call interrupted by disconnect"), no object exists and ~ProxyClientBase never runs. All the teardown lives in that destructor via m_context.cleanup_fns (release m_client on the loop thread and, for destroy_connection clients, delete the Connection), so the Connection and its EventLoopRef leak. EventLoop::loop() only returns when the reference count drops to zero.

The hang as observed depended on the other half of the same PR. By 2026-07-13 the branch registered the delete-on-disconnect handler only after the constructor returned (the fix for mp#308, the ConnectStream use-after-free), so a constructor that threw left nothing to delete the Connection. In the code before mp#298, ConnectStream registered that handler before constructing the client, so a construct() that failed because the peer was gone still had its Connection deleted when the disconnect fired. On that code the leak needs a construct() that throws on a live connection, for example an exception from the server's construct() implementation. Bitcoin Core's Init.construct server side only allocates a ProxyServer<ThreadMap> (include/mp/type-threadmap.h), so it does not throw in practice.

The missing cleanup dates from the initial commit, which already called construct() as the last constructor step with no cleanup on throw; c685fa9bedaf (2019) added the destroy_connection option that made the leaked resource the Connection itself.

The code

ProxyClientBase in include/mp/proxy-io.h, as of bb473690c97c^. The constructor registers the client's teardown as a cleanup function, which only the destructor runs, and makes the blocking construct() IPC call as its last step:

    m_context.cleanup_fns.emplace_front([this, destroy_connection, disconnect_cb]{
    {
        // ...
        m_context.loop->sync([&]() {
            // ...
            if (destroy_connection) {
                delete m_context.connection;
                m_context.connection = nullptr;
            }
        });
    }
    });
    Sub::construct(*this);
}

template <typename Interface, typename Impl>
ProxyClientBase<Interface, Impl>::~ProxyClientBase() noexcept
{
    MP_LOG(*m_context.loop, Log::Debug) << "Cleaning up " << CxxTypeName(*this) << " " << this;
    CleanupRun(m_context.cleanup_fns);

If Sub::construct(*this) throws, the constructor exits by exception, no object exists, and ~ProxyClientBase never runs, so the cleanup function that deletes the Connection is dropped without being called. The Connection and the EventLoopRef it holds leak, and EventLoop::loop(), which returns only when no references remain, never exits. Before this commit the delete-on-disconnect handler in ConnectStream still deleted the Connection when a dead peer made construct() fail; the leak needed a construct() that throws on a live connection. The hang in the test appeared once the same commit moved that handler to after construct() (see mp#308).

The fix runs the cleanup functions before rethrowing (bb473690c97c; comment, the mp#308 hunks and the tests omitted):

@@ -592,7 +592,28 @@ ProxyClientBase<Interface, Impl>::ProxyClientBase(typename Interface::Client cli
-    Sub::construct(*this);
+    try {
+        Sub::construct(*this);
+    } catch (...) {
+        MP_LOG(*m_context.loop, Log::Debug) << "Cleaning up " << CxxTypeName(*this) << " " << this << " after construct() failure";
+        CleanupRun(m_context.cleanup_fns);
+        throw;
+    }

A failed construct() now does the same teardown the destructor would have done, deleting the Connection and releasing its EventLoopRef, so the event loop can exit.

Fix

bb473690c97c wraps Sub::construct(*this) in try { } catch (...) { CleanupRun(m_context.cleanup_fns); throw; } and logs "Cleaning up ... after construct() failure" at debug level. In the fixed constructor the cleanup runs before the delete-on-disconnect handler is registered, so a failed construct never leaves a handler pointing at a connection the caller no longer knows about. The same commit fixes the mp#308 ConnectStream use-after-free; ryanofsky asked for both in one commit.

The commit message lists this among "Two bugs that have always been present", and mp#309 describes it as a disconnect-triggered hang. The leak on a throwing construct() has always been present, but the disconnect-triggered hang the test hit only existed on the PR branch, for the reason above.

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 Error paths that skip cleanup or completion: A constructor throw skips cleanup

  • Prevented by a clear ownership model, a review checklist item
  • Earliest detector a unit test (did not exist then)
  • Diagnosis easy
  • Fix design easy
  • Debugging aids a reproduction script, logging
  • Rust partly

Would Rust have prevented it? Partly. A new() that returns Err drops every field already built, so a Connection owned by the client struct would be freed on the failed construct() without any catch block; but the cleanup here is a list of closures run only by the destructor, and Rust drops an unrun closure list just as silently. More on Rust

Could it have been prevented?

Yes. The ProxyClientBase constructor registers its teardown (release the client on the loop thread, delete the Connection) as a lambda on m_context.cleanup_fns, a list that only ~ProxyClientBase runs, and then makes a blocking remote construct() call as its last step. A constructor that throws leaves no object, so anything not owned by a member with a destructor is dropped. Had the Connection been held through an RAII member (or the cleanup list itself been an object whose destructor runs it), the throw would have been safe without thought. The initial 2019 commit already had this shape, and the destroy_connection option added a month later made the leaked resource the whole connection and its EventLoopRef. A review item, "does every throw path in this constructor release what was acquired before it?", would have flagged 06e1045baba0's construct() call, since it is the only statement after the cleanup registration.

Could it have been found sooner?

Yes, by a unit test whose server construct() throws and which then expects the event loop to exit. No such test existed for seven years: the only construct() in use was Bitcoin Core's Init.construct, whose server side allocates a ProxyServer<ThreadMap> and cannot fail short of bad_alloc, so nothing ever exercised the throw path. The disconnected-socket test in mp#298 reached it only after xyzconstant added a FooInit interface with construct() so the test would follow the same path as Bitcoin Core, and only because the same branch had moved the delete-on-disconnect handler behind the constructor for the sibling mp#308 fix. A leak sanitizer would not have helped, since the hang comes before the process exits.

What made it hard to debug?

Not much. The failure was deterministic, a test that hung the whole mptest suite in TestSetup's destructor joining the loop thread, and the same-day comment already named the Connection leak and the unreleased EventLoopRef; the fix is an 11-line try/catch that runs the cleanup list before rethrowing (bb473690c97c). The one non-obvious step was knowing that EventLoop::loop() returns only when its reference count reaches zero, so a join that never returns means a ref is outstanding. A diagnostic that names the remaining refs when the loop is asked to exit, or a bounded join in the test harness, would turn a suite hang into a failure with a message. The fix's own debug line, "Cleaning up ... after construct() failure", is the log this path now leaves.

Lesson

Cleanup that is only run by a destructor does not run when the constructor throws; either own every acquired resource through a member with its own destructor, or wrap any fallible step after the acquisition and release before rethrowing.

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 hang
Worst effect observed hang
Seen in mptest
Scope production
Trigger Server disconnects abruptly (server-unclean-disconnect)
Phase startup
Side client
Component proxy-io
Root cause kind resource-leak
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