IPC stability bugs

Bug 47

Node aborts when a client sends overlapping calls to one server thread

Status
Fixed
Symptom
Assertion failure; worst effect seen: assertion failure
Where seen
A production node, Bitcoin Core functional tests
Trigger
Several calls running at the same time
First reported
2025-09-05 (first report)
Fixed
2025-10-02
Fix
mp#214
Shipped to Bitcoin Core
bitcoin#33518, bitcoin#33519
In released binaries
Yes (v30.0)
Defect ever on master
Yes

Summary

An IPC server aborted on Assertion '!m_fn' failed in Waiter::post when a client sent a second request for a server thread that already had a request waiting. The libmultiprocess C++ client never does this, but Python and Rust clients that reused one Context.thread for overlapping asynchronous calls could crash bitcoin-node. It was seen in Bitcoin Core's first Python IPC functional test and twice with a Rust Stratum v2 pool. Fixed in mp#214, which returns a "thread busy" error instead. v30.0 has the bug; the fix reached the 30.x branch afterwards.

What happened

Root cause

Each server worker thread has a Waiter, a small queue it waits on for its next call. Waiter::post in include/mp/proxy-io.h kept the function to run on the worker in a single slot, std::optional<kj::Function<void()>> m_fn, and asserted !m_fn before overwriting it. That matched the C++ client, which blocks on each call, so a server Thread never gets a new request while one is waiting. (One request can be executing and one waiting, since the worker clears m_fn before running it; that is why the regression test needs three calls to trigger it, as ryanofsky explained.) A non-C++ client that reuses a Context.thread capability for overlapping asynchronous calls breaks the assumption, and the event loop thread, posting the extra request from PassField in include/mp/type-context.h, aborted the process.

The single-slot design and the assertion date from the initial commit. Only non-C++ clients can reach it, and bitcoin#33201 in September 2025 added the first one to Bitcoin Core's test suite.

The code

Quoted from eb069ab75d83^. Each server worker thread has a Waiter (include/mp/proxy-io.h) with a single slot for the next function to run. post asserts the slot is empty, and the worker empties it just before running the function:

template <typename Fn>
void post(Fn&& fn)
{
    const Lock lock(m_mutex);
    assert(!m_fn);
    m_fn = std::forward<Fn>(fn);
    m_cv.notify_all();
}
// ...
        while (m_fn) {
            auto fn = std::move(*m_fn);
            m_fn.reset();
            Unlock(lock, fn);
        }

On the event-loop thread, PassField (include/mp/type-context.h) posts every request to the thread named in its Context.thread field, with no check that the thread is free:

const auto& thread = static_cast<ProxyServer<Thread>&>(*thread_server);
server.m_context.loop->log()
    << "IPC server post request  #" << req << " {" << thread.m_thread_context.thread_name << "}";
thread.m_thread_context.waiter->post(std::move(invoke));

The C++ client blocks on each call, so it never sends a second request on the same thread while the first is still queued. A client in another language can, by reusing one Context.thread for overlapping asynchronous calls:

  1. Event loop: posts request 1; the worker takes it out of m_fn and starts running it.
  2. Event loop: posts request 2 into the now-empty slot, where it waits.
  3. Event loop: posts request 3; m_fn is still set and assert(!m_fn) aborts the whole server process.

The fix (eb069ab75d83) turns the assertion into a return value, and PassField fails only that request:

@@ -281,12 +281,13 @@ struct Waiter
-    void post(Fn&& fn)
+    bool post(Fn&& fn)
     {
         const Lock lock(m_mutex);
-        assert(!m_fn);
+        if (m_fn) return false;
         m_fn = std::forward<Fn>(fn);
         m_cv.notify_all();
+        return true;
     }
@@ -152,7 +152,12 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn&
-                thread.m_thread_context.waiter->post(std::move(invoke));
+                if (!thread.m_thread_context.waiter->post(std::move(invoke))) {
+                    server.m_context.loop->log()
+                        << "IPC server error request #" << req
+                        << " {" << thread.m_thread_context.thread_name << "}" << ", thread busy";
+                    throw std::runtime_error("thread busy");
+                }

The exception is thrown inside the promise continuation for that request, so Cap'n Proto returns it to the client as a remote error for request 3 and the server keeps running.

Fix

The fix commit eb069ab75d83 (@Eunovo) makes post return false when m_fn is already set; PassField then logs "IPC server error request #N {thread}, thread busy" and throws std::runtime_error("thread busy"), which the client receives as a remote exception (+9/-3). The test "Make simultaneous IPC callbacks with same request_thread and callback_thread" (1238170f68e8) sends three requests on one thread with the raw Cap'n Proto client and expects the error; it is based on Eunovo's Python reproducer.

mp#240 ("Avoid errors from asynchronous (non-c++) clients", 2026-02) later replaced the error with a per-thread request queue, after which mp#206 was closed.

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 in-process interface trusting what now comes from another process: An assertion on what is now input from a peer

  • Prevented by a design rule, a review checklist item
  • Earliest detector a test with a non-C++ client (did not exist then)
  • Diagnosis easy
  • Fix design easy
  • Debugging aids a clear assertion message, logging
  • Rust no

Would Rust have prevented it? No. A protocol assumption (one outstanding request per thread) violated by a foreign client is a contract bug; in Rust the same assert! would panic and the same fix, returning an error, is a design choice. More on Rust

Could it have been prevented?

Yes. Waiter::post asserted !m_fn before storing the next request for a server thread, which encoded the C++ client's behaviour (one blocking call per thread at a time) as an invariant of the server. From the day a non-C++ client could name a Context.thread capability, whether a second request arrives for that thread is the peer's choice, and an assert on the peer's choice is a remote abort of bitcoin-node. The rule is the same as for bitcoin#33222: at a wire boundary, a precondition on what the peer sends is input validation, and the fix (eb069ab75d83) is exactly that, post returns false and the request fails with "thread busy". The review item is mechanical: list every assert reachable from a request's contents. The assertion dates from the initial commit, when the only client was the library's own; it became input the day bitcoin#30510 put the mining interface behind -ipcbind.

Could it have been found sooner?

Yes, by any client that does not serialize its calls. The C++ client cannot send two requests on one thread, so no C++ test could reach the assert in six years; the first pycapnp test in Bitcoin Core (bitcoin#33201) reached it within days, by accident, when an un-awaited waitNext left a second call in flight. Eunovo's regression test drives the raw messages::FooInterface::Client with three requests on one thread, which is the shape of test that would have found it at any point: a raw Cap'n Proto client in the library's own suite. The Rust Stratum v2 client hit the same abort in the field a month later on v30.0rc3 (bitcoin#33554).

What made it hard to debug?

Little. The CI failure was intermittent (about one run in sixty) and the Python side only hung, but the node's stderr carried the assert with file and line, and ryanofsky named the cause on IRC within minutes and opened mp#206 the same day. The "IPC server post request #N {thread}" log line placed the extra request on the thread. What took a month was the fix's shape: returning an error was quick, and the question of whether the server should instead queue requests per thread was settled later by mp#240, which is where the next regressions came from.

Lesson

An assertion that holds because of how your own client behaves is a crash waiting for the first client written by someone else; when an interface goes over the wire, turn every such assert into a rejected request, and add a raw-protocol client to the test suite so the "impossible" sequences are sent on purpose.

Classification

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

Field Value
Symptom assert
Worst effect observed assert
Seen in production-node, functional-test
Scope production
Trigger Concurrent calls (concurrent-calls)
Phase steady-state
Side server
Component proxy-io
Root cause kind assumption-violated
Fix kind local-fix
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