IPC stability bugs

Bug 52

Ctrl-C cannot stop the node while a mining client waits for a new tip

Status
Fixed
Symptom
Hang; worst effect seen: hang
Where seen
A production node
Trigger
A signal such as Ctrl-C
First reported
2025-05-01 (first report)
Fixed
2025-11-12
Fix
bitcoin#33511
In released binaries
Yes (v30.0-v30.1)
Defect ever on master
Yes

Summary

bitcoin-node could not be stopped with Ctrl-C or SIGTERM while an IPC client had a Mining.waitTipChanged call (wait until the chain tip changes) outstanding: shutdown printed "Shutdown: done" and the process then hung until killed. The signal path never woke the condition variable the call was waiting on, so a client that used a long timeout kept the IPC event loop alive forever. Affected v30.0 and v30.1; fixed on master by bitcoin#33511 ("init: Fix Ctrl-C shutdown hangs during wait calls") and backported to the 30.x branch in bitcoin#34192 (released in v30.2).

What happened

Root cause

WaitTipChanged (src/node/miner.cpp) waits on KernelNotifications::m_tip_block_cv until the tip changes or chainman.m_interrupt is set. On shutdown the only code that notified that condition variable was in NodeContext::shutdown_request (src/init.cpp), which the stop RPC calls. The Ctrl-C / SIGTERM handler HandleSIGTERM only trips the g_shutdown SignalInterrupt, so m_interrupt became true but nothing woke the waiter, and the worker thread stayed in wait_until until the client's deadline.

That worker holds the request and an EventLoopRef (a reference that keeps the event loop running), so disconnectIncoming(), which closes client connections during shutdown, could not finish tearing down the connection, the event loop stayed alive, and ~CapnpProtocol blocked on m_loop_thread.join(). By then Shutdown() had already destroyed the KernelNotifications and ChainstateManager objects the waiting worker was still using (ryanofsky's analysis of the threads).

The missing wake-up is not IPC-specific: Ctrl-C did not interrupt the waitforblockheight, waitforblock and waitfornewblock RPCs either, but RPC callers use short timeouts and just wait them out. Only an IPC client parks a thread there with a near-infinite timeout.

The code

Quoted from c25a5e670b27^. The IPC worker running waitTipChanged sleeps here, in WaitTipChanged (src/node/miner.cpp), until the condition variable is notified or the deadline passes:

if (timeout > std::chrono::years{100}) timeout = std::chrono::years{100}; // Upper bound to avoid UB in std::chrono
auto deadline{std::chrono::steady_clock::now() + timeout};
{
    WAIT_LOCK(kernel_notifications.m_tip_block_mutex, lock);
    // ...
    kernel_notifications.m_tip_block_cv.wait_until(lock, deadline, [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) {
        return Assume(kernel_notifications.TipBlock()) != current_tip || chainman.m_interrupt;
    });
}

The only shutdown code that notifies it is node.shutdown_request in InitContext (src/init.cpp), which the stop RPC calls. The signal handler trips the same g_shutdown flag directly and skips the notification:

    node.shutdown_request = [&node] {
        assert(node.shutdown_signal);
        if (!(*node.shutdown_signal)()) return false;
        // Wake any threads that may be waiting for the tip to change.
        if (node.notifications) WITH_LOCK(node.notifications->m_tip_block_mutex, node.notifications->m_tip_block_cv.notify_all());
        return true;
    };
// ...
static void HandleSIGTERM(int)
{
    // Return value is intentionally ignored because there is not a better way
    // of handling this failure in a signal handler.
    (void)(*Assert(g_shutdown))();
}

chainman.m_interrupt is that same g_shutdown object, so after Ctrl-C the predicate would return true, but nothing wakes the thread to evaluate it:

  1. IPC worker: WaitTipChanged sleeps in wait_until with a deadline up to 100 years away (the client passed 1.8e30 ms).
  2. Signal: HandleSIGTERM sets g_shutdown; no notify_all().
  3. Main thread: Interrupt() and Shutdown() run and do not notify either, then ~CapnpProtocol waits for the event loop, which the sleeping worker keeps alive.

The fix moves the notification from shutdown_request into Interrupt(), which the main thread runs after either kind of shutdown request (c25a5e670b27; the src/node/interfaces.cpp hunks that make Qt call Interrupt() are omitted):

@@ -215,8 +215,6 @@ void InitContext(NodeContext& node)
     node.shutdown_request = [&node] {
         assert(node.shutdown_signal);
         if (!(*node.shutdown_signal)()) return false;
-        // Wake any threads that may be waiting for the tip to change.
-        if (node.notifications) WITH_LOCK(node.notifications->m_tip_block_mutex, node.notifications->m_tip_block_cv.notify_all());
         return true;
     };
 }
@@ -267,6 +265,8 @@ void Interrupt(NodeContext& node)
 #if HAVE_SYSTEM
     ShutdownNotify(*node.args);
 #endif
+    // Wake any threads that may be waiting for the tip to change.
+    if (node.notifications) WITH_LOCK(node.notifications->m_tip_block_mutex, node.notifications->m_tip_block_cv.notify_all());
     InterruptHTTPServer();

bitcoind calls Interrupt(node) as soon as shutdown_signal->wait() returns, whatever set the signal, so the waiting worker now wakes, sees m_interrupt and returns, the IPC call completes, and the event loop can exit.

Fix

Commit c25a5e670b27 ("init: Signal m_tip_block_cv on Ctrl-C") moves m_tip_block_cv.notify_all() from shutdown_request to Interrupt(), which runs on both the stop and the signal shutdown paths. It also makes the Qt startShutdown() call Interrupt() instead of its own InterruptRPC()/StopRPC() pair, updating the earlier Qt-only fix from bitcoin#18452 ("qt: Fix shutdown when waitfor* cmds are called from RPC console"). An intermediate version of the PR regressed GUI console waitfor* calls; @Sjors caught it and it was corrected before merge. Commit 6a29f79006a9 adds a feature_init.py test that sends SIGTERM (Ctrl-Break on Windows) during a long waitforblockheight.

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: An interrupt that wakes nobody

  • Prevented by a different API shape, a design rule
  • Earliest detector a test that kills a process (did not exist then)
  • Diagnosis moderate
  • Debugging aids symbolized stack traces, logging
  • Rust no

Would Rust have prevented it? No. A condition variable whose notify lives on one of two shutdown paths is a missed-wakeup logic bug; Rust's Condvar has the same split between the flag and the notification. More on Rust

Could it have been prevented?

Yes, by shape. WaitTipChanged waits on a condition variable whose predicate tests chainman.m_interrupt, but the flag and the wake-up live in different objects: the SignalInterrupt is tripped by the signal handler, the notify_all() by whichever shutdown function someone last put it in. The refactor that split shutdown_signal from shutdown_request (5ca28ef28bcc) kept the notify on one of the two paths, and nothing complained. If the interrupt object itself woke registered condition variables (or the wait used an interruptible primitive that takes the interrupt as a parameter), there would be no second place to forget. The fix is a two-line move into Interrupt(), the one function both paths run; the rule it implies is that every wake-up for an interrupt flag belongs on the path the flag is set, not on the path a caller happens to use.

Could it have been found sooner?

Yes. The test that ships with the fix (SIGTERM during a long waitforblockheight in feature_init.py) needs no IPC at all and fails deterministically; feature_init.py existed since 2022 but only tested interrupts during startup. Nobody wrote a shutdown variant because RPC callers use short timeouts and would have waited them out; the combination that made the hang visible (a thread parked for 1.8e30 ms, then Ctrl-C) took an external mining client. pinheadmz did see it seven months after introduction (2025-05-01, bitcoin-cli -ipcconnect in waitforblock) but it was put down to the shutdown problems bitcoin#32345 was fixing, so the first report was lost in the noise of concurrent bugs.

What made it hard to debug?

Finding it was the slow part, not explaining it: once plebhash posted thread backtrace all (2025-09-29), ryanofsky's annotated four-thread analysis and a one-line diff followed the next day. Two false leads cost time: the earlier misattribution to bitcoin#32345, and ryanofsky's first guess that the bitcoin#33387 barrier patch would fix it, which testing disproved. The fix's own commit message names the wrong introducing PR, which the survey corrected only in 2026. What would have shortened it: shutdown logging that says what is keeping the event loop alive (the EventLoopRef count and the request holding it are already known to the loop), so a "Shutdown: done" followed by silence points at the worker in waitTipChanged without a debugger.

Lesson

A wait whose predicate checks an interrupt flag is only as safe as the guarantee that setting the flag notifies the waiter; put the notify where the flag is set, and test the signal path, not just the polite one.

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 production-node
Scope production
Trigger Signal (Ctrl-C) (signal)
Phase shutdown
Side server
Component bitcoin-init
Root cause kind missing-cancellation
Fix kind local-fix
Detection manual-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