Bug 95

Concurrent first IPC use can abort the process

Status
Open
Symptom
Crash; worst effect seen: crash
Where seen
Bitcoin Core unit tests, Bitcoin Core functional tests
Trigger
Several calls running at the same time
First reported
Reported privately in an independent security review, 2026-09-27
Fixed
Not yet
In released binaries
Yes (v30.0 onward)
Defect ever on master
Yes

Summary

Bitcoin Core starts its IPC event loop on a background thread the first time a process uses IPC. The check for "is the loop already running?" has no lock, so if two threads make their first IPC call at the same time, both can start a loop thread, and replacing the first, still-running thread makes C++ terminate the process. It was reported privately in an independent security review with a test that forces the timing. On master the only way to make two first calls at once is the hidden, test-only echoipc RPC on a bitcoin-node started without -ipcbind. No fix is merged yet.

What happened

Nothing has been seen outside the reporter's tests. An independent security review reported the problem privately on 2026-09-27. Its unit test holds the first loop thread just before it creates the loop, lets a second caller pass the same check, and the report says the process then aborted in std::terminate. A day later the reporter added a reproducer at the process level: eight echoipc RPC calls sent at once to a fresh bitcoin-node built with ThreadSanitizer, which reported two data races on master. Neither run has been repeated independently.

Root cause

CapnpProtocol::startLoop() in src/ipc/capnp/protocol.cpp runs at the start of connect(), listen() and makeStream(). If the m_loop member is already set it returns; otherwise it starts a thread that constructs the mp::EventLoop into m_loop, signals the caller through a std::promise, and runs the loop. The caller waits for the signal.

Nothing stops two callers from running this at once. m_loop is only set after the new thread has started, so a second caller that arrives in between also sees no loop and starts a second thread. Assigning that thread to m_loop_thread, which still holds the first thread, calls std::terminate: a std::thread that is still joinable may not be replaced. A second caller that arrives a little later can see m_loop while the loop thread is still writing it, and use a loop that is not finished; that is the second race ThreadSanitizer reported.

The code assumes the first IPC use in a process happens on one thread. That is true when bitcoin-node -ipcbind starts the loop during startup, before any RPC call is accepted, and in bitcoin-cli. It is not true for spawnProcess(), which echoipc calls from whichever RPC worker thread handles the request.

Fix

No fix merged yet. The fix proposed with the report adds a mutex that startLoop() holds for the whole check, thread start and wait, so a second caller waits for the first and then reuses its loop:

     void startLoop()
     {
+        const std::lock_guard lock{m_loop_mutex};
         if (m_loop) return;

The loop thread never takes the mutex, so the wait cannot deadlock. The change is built on the fix for Process aborts instead of reporting an error when the IPC event loop fails to start, which touches the same function.

Next steps

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 Locks, waits and wakeups: Unsynchronized lazy init

  • Prevented by lock annotations
  • Earliest detector a stress test (did not exist then)
  • Diagnosis easy
  • Fix design easy
  • Debugging aids sanitizer output
  • Rust yes

Would Rust have prevented it? Yes. Lazily starting shared state from several threads needs &self plus interior mutability in Rust, which the compiler only accepts behind a Mutex, OnceLock or similar, so the unlocked check-then-start would not compile. More on Rust

Could it have been prevented?

Yes. A lazily initialised member that more than one thread can reach needs a lock or a once-only primitive around the check and the initialisation together. startLoop() in 10afdf0280fa was written when every caller started the loop once from one thread, so the unguarded check was safe by convention rather than by construction. Annotating m_loop as guarded by a mutex (Clang's thread safety analysis) would have flagged the unguarded read and write. The fix shows the rule: one mutex held for the check, the start and the wait.

Could it have been found sooner?

Yes, by a test that makes two first calls at once, ideally under ThreadSanitizer. No such test existed: the IPC tests and rpc_misc.py start the loop once, from one thread. The only concurrent path on master is a test RPC, so ordinary use would not have hit it. An independent security review found it by reading the code and confirmed it with barriers that force the interleaving.

What made it hard to debug?

Little. The abort comes with "terminate called without an active exception", which is vague, but ThreadSanitizer names the unguarded member directly, and the barrier test makes the timing deterministic.

Lesson

"Start it on first use" code is shared state like any other: if more than one thread can be the first user, the check and the start need one lock, and a test that makes two first calls at once.

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 unit-test, functional-test
Scope production
Trigger Concurrent calls (concurrent-calls)
Phase steady-state
Side eventloop
Component bitcoin-ipc
Root cause kind missing-lock
Fix kind none
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