Bug 57

Node terminates when a call on a worker thread throws, e.g. on bad input

Status
Fixed
Symptom
Crash; worst effect seen: crash
Where seen
Bitcoin Core functional tests
Trigger
A client that does not use libmultiprocess, such as a Python or Rust client
First reported
2025-09-08 (first report)
Fixed
2026-02-20
Fix
mp#240
Shipped to Bitcoin Core
bitcoin#34422
In released binaries
Yes (v30.0-v30.3)
Defect ever on master
Yes

Summary

bitcoin-node terminated when an IPC method running on a server worker thread threw an exception, for example while deserializing a malformed argument such as an empty coinbase passed to BlockTemplate.submitSolution. The exception escaped the worker thread and std::terminate ended the process, instead of an error being returned to the client. Any client able to send malformed arguments (in practice Python or Rust clients) could crash the node. Fixed in mp#240 ("Avoid errors from asynchronous (non-c++) clients") and shipped in Bitcoin Core v31; v30.0 through v30.3 have the bug.

What happened

Root cause

Methods with an mp.Context argument run on a server worker thread. PassField in include/mp/type-context.h built a lambda that read the remaining Cap'n Proto parameters and called the method via fn.invoke(server_context, args...), and wrapped only the promise-fulfilling code after it in kj::runCatchingExceptions. Here the coinbase bytes are converted to a CTransactionRef by Bitcoin Core's CustomReadField, which throws std::ios_base::failure on truncated data. The lambda ran on a std::thread created by ProxyServer<ThreadMap>::makeThread, which has no handler, so the exception left the thread and called std::terminate. The try/catch in serverInvoke (include/mp/proxy-types.h) only covers methods run on the event loop thread, which is why methods without a context argument returned errors normally. An exception from the method body itself on a worker thread crashed the same way.

This path has been unguarded since the initial commit. The C++ client serializes arguments from valid C++ objects, so arguments that fail to deserialize were practically impossible until Python and Rust clients could build requests by hand.

The code

PassField in include/mp/type-context.h, as of c4762c7b513a^. The invoke lambda runs on the worker thread; fn.invoke reads the remaining parameters (here submitSolution's coinbase bytes) and calls the method:

auto invoke = [fulfiller = kj::mv(future.fulfiller),
     call_context = kj::mv(server_context.call_context), &server, req, fn, args...]() mutable {
            // ...
                fn.invoke(server_context, args...);
            }
            KJ_IF_MAYBE(exception, kj::runCatchingExceptions([&]() {
                server.m_context.loop->sync([&] {
                    auto fulfiller_dispose = kj::mv(fulfiller);
                    fulfiller_dispose->fulfill(kj::mv(call_context));
                });
            }))

The lambda is run by Waiter::wait on the thread that ProxyServer<ThreadMap>::makeThread (src/mp/proxy.cpp) starts, and nothing on that thread catches:

std::thread thread([&thread_context, from, this]() {
    // ...
    Lock lock(g_thread_context.waiter->m_mutex);
    // Wait for shutdown signal from ProxyServer<Thread> destructor (signal
    // is just waiter getting set to null.)
    g_thread_context.waiter->wait(lock, [] { return !g_thread_context.waiter; });
});

kj::runCatchingExceptions only covers the call that fulfills the promise. When deserializing an argument throws (Bitcoin Core's CTransactionRef reader throws std::ios_base::failure on the empty coinbase), the exception leaves fn.invoke, unwinds out of the thread function and std::terminate ends the process. The try/catch in serverInvoke is on the event-loop thread and never sees it.

The fix, labelled a refactor, moves posting into a new ProxyServer<Thread>::post() that runs the whole invoke inside kj::runCatchingExceptions and rejects the promise with what it catches (c4762c7b513a; the call-site changes, logging and constructor changes are omitted):

--- a/include/mp/proxy-io.h
+++ b/include/mp/proxy-io.h
@@ -675,6 +689,31 @@ struct ThreadContext
+        bool posted = m_thread_context.waiter->post([this, fn = std::forward<Fn>(fn), result_fulfiller = kj::mv(result.fulfiller)]() mutable {
+            std::optional<T> result_value;
+            kj::Maybe<kj::Exception> exception{kj::runCatchingExceptions([&]{ result_value.emplace(fn()); })};
+            m_loop->sync([&result_value, &exception, result_fulfiller = kj::mv(result_fulfiller)]() mutable {
+                KJ_IF_MAYBE(e, exception) {
+                    assert(!result_value);
+                    result_fulfiller->reject(kj::mv(*e));
+                } else {
--- a/include/mp/type-context.h
+++ b/include/mp/type-context.h
@@ -125,18 +124,7 @@
                     fn.invoke(server_context, args...);
                 }
-                KJ_IF_MAYBE(exception, kj::runCatchingExceptions([&]() {
-                    server.m_context.loop->sync([&] {
-                        auto fulfiller_dispose = kj::mv(fulfiller);
-                        fulfiller_dispose->fulfill(kj::mv(call_context));
-                    });
-                }))
+                return call_context;

An exception from argument reading or from the method body is now caught on the worker thread and sent back to the client as a remote exception instead of terminating the server.

Fix

The fix commit c4762c7b513a moves the posting code into a new ProxyServer<Thread>::post() in include/mp/proxy-io.h, whose worker-side lambda runs kj::runCatchingExceptions([&]{ result_value.emplace(fn()); }) around the whole call and rejects the request's promise with the caught exception (+51/-31). The client now gets remote exception: std::exception: SpanReader::read(): end of data: iostream error.

The commit message calls this a refactor that "does not change behavior other than changing log messages". The diff says otherwise: before it, fn.invoke ran outside runCatchingExceptions; after it, the whole call is inside. That change is what fixes this crash, and ryanofsky named this commit as the fix on the issue. A test, "Submit solution that can't be deserialized", was added to Bitcoin Core in bitcoin#34727 ("test: Add IPC wake-up test and reuse mining context").

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: An exception escaping a thread

  • Prevented by a different API shape, a review checklist item
  • Earliest detector a unit test (did not exist then)
  • Diagnosis easy
  • Fix design easy
  • Debugging aids a reproduction script, a clear assertion message
  • Rust partly

Would Rust have prevented it? Partly. Deserialization failure would be a Result the worker must handle, and a panic on a spawned thread ends that thread rather than the process, but the request's promise would then never be fulfilled, so the crash becomes a dead worker and a hung client unless the panic is caught and turned into a reply, as the fix does with kj::runCatchingExceptions. More on Rust

Could it have been prevented?

Yes. The event-loop path already had the right shape: serverInvoke catches whatever the method throws and turns it into a rejected promise. The worker-thread path in PassField did the same job by hand and wrapped only the fulfiller call in kj::runCatchingExceptions, leaving fn.invoke (argument reading plus the method body) uncovered on a std::thread with no handler, since the initial commit (06e1045baba0). The fix's API shape is the prevention: a single ProxyServer<Thread>::post() (c4762c7b513a) that owns the catch and the reject, so a caller cannot dispatch to a worker without it. A review checklist line, "every function run as a thread's top frame or posted to another thread converts exceptions to an error result", names the same thing and is cheap to apply; the duplicated catch logic was the tell.

Could it have been found sooner?

Yes, by a unit test in which a Proxy.Context method throws from its body and the client expects a remote exception. mptest had many context-taking methods from 2019 but none that threw, and the C++ client cannot send an argument that serializes but does not deserialize, so the deserialization form was unreachable in-tree. The first thing that could reach it was a hand-built request: the pycapnp client in interface_ipc.py (bitcoin#33201, 2025-09) and fuzzamoto driving it found the crash within days (bitcoin#33341). The test that now covers it, "Submit solution that can't be deserialized", came a month after the fix in bitcoin#34727; the fix commit itself was labelled a behavior-preserving refactor and carried no test.

What made it hard to debug?

It was not. The terminate called after throwing ... SpanReader::read(): end of data message and a deterministic one-line reproduction were in the first report; Sjors showed the same day that the throw preceded the method body, and ryanofsky placed it in the worker dispatch path. The delay was priority, not diagnosis: the fix waited five months for the mp#240 rework of that path, and because the commit called itself a refactor, the issue was only closed after ryanofsky pointed at it. A commit that fixes a crash should say so; a later reader of bitcoin#34644 hit the same abort on a pre-fix branch and needed that attribution.

Lesson

Every thread entry point and every hand-off to another thread is a place where an exception has no caller to reach; put the catch-and-convert in the one function that does the hand-off, and test it with a callee that throws, before an untrusted peer supplies the throw for you.

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 functional-test
Scope production
Trigger Non-C++ client (non-cpp-client)
Phase steady-state
Side server
Component type-context
Root cause kind unhandled-exception
Fix kind local-fix
Detection fuzz
Needs a peer that is not libmultiprocess C++ yes
Needed a sanitizer to see no
Defect ever on master yes
Released binary
Confidence high