IPC stability bugs

Bug 63

Thread waits forever when a function it posted to the event loop throws

Status
Fixed
Symptom
Deadlock; worst effect seen: nothing observed
Where seen
Never observed
Trigger
Unusual input from a peer or caller
First reported
2026-03-19 (first report)
Fixed
2026-04-14
Fix
mp#260
Shipped to Bitcoin Core
bitcoin#35661
In released binaries
Yes
Defect ever on master
Yes

Summary

EventLoop::post() is how another thread runs a function on the event loop thread and waits for it to finish. If a function passed to it threw an exception, the thread that called post() blocked forever, because the event loop skipped the code that marks the call finished and wakes the caller. No caller in libmultiprocess or Bitcoin Core lets a posted function throw, so this was never observed in practice; it was found by code reading and shown with a purpose-built reproducer. Fixed in mp#260 by catching and logging the exception. The code was present in v30.0 through v31.1 with no known way to trigger it.

What happened

Never observed in practice. @hulxv reported in mp#259 (2026-03-19) that if a function handed to EventLoop::post() throws, the calling thread never returns: it waits on m_cv for m_post_fn to change, and nothing resets m_post_fn or notifies. The report came with a standalone reproducer run under Helgrind, not a failing test or CI job. @Sjors asked whether mp#249, a set of fixes for race conditions on disconnect then under review, covered it; hulxv confirmed they did not.

Root cause

EventLoop::loop() in src/mp/proxy.cpp ran the posted function and then cleared it:

Unlock(lock, *m_post_fn);
m_post_fn = nullptr;
m_cv.notify_all();

with no exception guard. An exception from the function unwound out of loop(), skipping both statements and the loop's cleanup, so the thread blocked in post() waited forever, and destroying the EventLoop afterwards would hit KJ_ASSERT(m_post_fn == nullptr) in ~EventLoop. This shape dates from the initial commit.

The issue (mp#259) says existing callers are safe because clientInvoke (the code that sends a client's call from the event loop) captures exceptions in a std::exception_ptr. That only holds for its response handler: the request-building part of the same posted function (get_request, handleChain with BuildParams, request.send()) has no try/catch, and is safe only because no current field builder throws.

The code

Quoted from b8a48c65e600^ (src/mp/proxy.cpp). A thread that is not the event-loop thread runs code there with EventLoop::post(), which stores a pointer to the function, wakes the loop and waits until the loop clears the pointer:

Lock lock(m_mutex);
EventLoopRef ref(*this, &lock);
m_cv.wait(lock.m_lock, [this]() MP_REQUIRES(m_mutex) { return m_post_fn == nullptr; });
m_post_fn = &fn;
// ...
m_cv.wait(lock.m_lock, [this, &fn]() MP_REQUIRES(m_mutex) { return m_post_fn != &fn; });

EventLoop::loop() runs the function and only then clears the pointer and notifies:

Lock lock(m_mutex);
if (m_post_fn) {
    Unlock(lock, *m_post_fn);
    m_post_fn = nullptr;
    m_cv.notify_all();

Nothing catches an exception from the posted function, so it unwinds out of loop() past the two lines after it. m_post_fn still points at the function and no notification is sent, so the posting thread waits forever (holding an EventLoopRef), and any later post() waits for m_post_fn == nullptr. If the EventLoop is destroyed anyway, ~EventLoop fails KJ_ASSERT(m_post_fn == nullptr). No current caller is known to throw here.

The fix catches and logs the exception so the loop always clears the pointer, notifies, and keeps running (b8a48c65e600; #include hunk omitted):

@@ -245,7 +246,12 @@ void EventLoop::loop()
         if (m_post_fn) {
-            Unlock(lock, *m_post_fn);
+            // m_post_fn throwing is never expected. If it does happen, the caller
+            // of EventLoop::post() will return without any indication of failure,
+            // which will likely cause other bugs. Log the error and continue.
+            KJ_IF_MAYBE(exception, kj::runCatchingExceptions([&]() MP_REQUIRES(m_mutex) { Unlock(lock, *m_post_fn); })) {
+                MP_LOG(*this, Log::Error) << "EventLoop: m_post_fn threw: " << kj::str(*exception).cStr();
+            }
             m_post_fn = nullptr;
             m_cv.notify_all();

m_post_fn = nullptr and notify_all() now run on every path, so the waiting thread returns from post(); as the new comment says, it gets no sign that the function failed.

Fix

b8a48c65e600 (hulxv) wraps the call in kj::runCatchingExceptions, logs "EventLoop: m_post_fn threw: ..." at Log::Error, and then resets m_post_fn, notifies and continues serving I/O (+7/-1). No test was added.

ryanofsky said "I'm not sure this is a bug exactly, but I think it's possible behavior could be improved here." In the merge review he noted that post() now returns without signalling failure, which "could potentially be worse than hanging, but the difference is unlikely to be significant", and suggested rethrowing the exception in the posting thread as a follow-up; the code comment in the fix says the same. hulxv's first version instead waited for clients to be released before propagating the exception, which ryanofsky pointed out could trade one deadlock for another.

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 throw skips the done signal

  • Prevented by a design rule, a different API shape
  • Earliest detector a unit test (did not exist then)
  • Diagnosis easy
  • Fix design moderate
  • Debugging aids a reproduction script
  • Rust no

Would Rust have prevented it? No. A callback failure that skips the notify is a control-flow bug: a Rust panic unwinding out of the loop body would poison m_mutex but the poster is already inside Condvar::wait and stays there, so the hang is the same. More on Rust

Could it have been prevented?

Yes. EventLoop::loop() ran the posted function and then, on the next two lines, cleared m_post_fn and notified the waiting thread; an exception skipped both, so the completion signal depended on the callee returning normally. The rule is that a completion signal owed to another thread is sent from a scope guard or finally, never from straight-line code after the call, and the library already had the idiom (TryFinally, KJ_DEFER) elsewhere. The API shape that makes it structural is a post() whose result carries success or an exception_ptr, so that "run this on the loop" and "tell the poster what happened" are one operation; ryanofsky suggested rethrowing in the posting thread as the follow-up, which is that shape. Whether it counts as a bug at all was contested, since no caller throws, and the merged fix (b8a48c65e600) trades a hang for a silent return; the comment it adds says so.

Could it have been found sooner?

By a one-line unit test that posts a throwing function and expects post() to return (or rethrow). None existed in seven years because nothing posted ever threw, and none was added with the fix. The other route is review with the question "what happens on this hand-off if the callee throws?", which is how hulxv found it (mp#259), with a standalone reproducer under Helgrind rather than a project test. The same question applied to clientInvoke shows a sibling: the request-building half of its posted lambda has no try/catch either, so after the fix a throw there would leave the caller waiting in clientInvoke instead of in post(); it stays latent for the same reason.

What made it hard to debug?

Nothing to debug, since it was never observed; what was hard was agreeing on the fix. hulxv's first version waited for clients to be released before propagating, which ryanofsky pointed out could trade one deadlock for another, and the log-and-continue version that merged has the drawback its own comment states. Had this ever hung a real process, the symptom would have been a thread parked in post() with the event loop still serving I/O, and a thread dump with the b-capnp-loop name would have shown the poster waiting and the loop idle; the KJ_ASSERT(m_post_fn == nullptr) in ~EventLoop would have fired only at shutdown, far from the cause.

Lesson

When one thread waits for another to finish a piece of work, the "done" signal must be sent on every exit path of that work, including exceptions; put it in a scope guard and give the waiter a way to learn that the work failed.

Classification

The survey's classification of this bug. The explorer charts these fields across all bugs; the method page explains them.

Field Value
Symptom deadlock
Worst effect observed none
Seen in never
Scope production
Trigger Unusual input (unusual-input)
Phase any
Side eventloop
Component eventloop
Root cause kind unhandled-exception
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 medium