From 6a29f79006a9d60b476893dface5eea8f9bf271c Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 30 Sep 2025 14:47:17 -0400 Subject: [PATCH 1/2] test: Test SIGTERM handling during waitforblockheight call Currently when CTRL-C is pressed and there is an active `waitforblockheight`, or `waitforblock`, or `waitfornewblock` RPC call, or a mining interface `waitTipChanged` IPC call with a long timeout, the node will not shut down right away, and will wait for the timeout to be reached before exiting. This behavior is not ideal and only happens when the node is stopped with CTRL-C or SIGTERM. When the node is stopped with `bitcoin-cli stop`, the wait calls are interrupted and the node does shut down right away. The next commit improves node behavior. This commit just adds test coverage to simplify the next commit and clarify the change in behavior there. --- test/functional/feature_init.py | 56 +++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index b9d41a9713c..d8a773bbe88 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -3,13 +3,16 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Tests related to node initialization.""" +from concurrent.futures import ThreadPoolExecutor from pathlib import Path import os import platform import shutil import signal import subprocess +import time +from test_framework.authproxy import JSONRPCException from test_framework.test_framework import BitcoinTestFramework from test_framework.test_node import ( BITCOIN_PID_FILENAME_DEFAULT, @@ -240,9 +243,62 @@ class InitTest(BitcoinTestFramework): self.stop_node(0) assert not custom_pidfile_absolute.exists() + def break_wait_test(self): + """Test what happens when a break signal is sent during a + waitforblockheight RPC call with a long timeout. Ctrl-Break is sent on + Windows and SIGTERM is sent on other platforms, to trigger the same node + shutdown sequence that would happen if Ctrl-C were pressed in a + terminal. (This can be different than the node shutdown sequence that + happens when the stop RPC is sent.) + + Currently when the break signal is sent, it does not interrupt the + waitforblockheight RPC call, and the node does not exit until it times + out.""" + + self.log.info("Testing waitforblockheight RPC call followed by break signal") + node = self.nodes[0] + + if platform.system() == 'Windows': + # CREATE_NEW_PROCESS_GROUP prevents python test from exiting + # with STATUS_CONTROL_C_EXIT (-1073741510) when break is sent. + self.start_node(node.index, creationflags=subprocess.CREATE_NEW_PROCESS_GROUP) + else: + self.start_node(node.index) + + current_height = node.getblock(node.getbestblockhash())['height'] + + with ThreadPoolExecutor(max_workers=1) as ex: + # Call waitforblockheight with wait timeout longer than RPC timeout, + # so it is possible to distinguish whether it times out or returns + # early. If it times out it will throw an exception, and if it + # returns early it will return the current block height. + self.log.debug(f"Calling waitforblockheight with {self.rpc_timeout} sec RPC timeout") + fut = ex.submit(node.waitforblockheight, height=current_height+1, timeout=self.rpc_timeout*1000*2) + time.sleep(1) + + self.log.debug(f"Sending break signal to pid {node.process.pid}") + if platform.system() == 'Windows': + # Note: CTRL_C_EVENT should not be sent here because unlike + # CTRL_BREAK_EVENT it can not be targeted at a specific process + # group and may behave unpredictably. + node.process.send_signal(signal.CTRL_BREAK_EVENT) + else: + # Note: signal.SIGINT would work here as well + node.process.send_signal(signal.SIGTERM) + node.process.wait() + + try: + result = fut.result() + raise Exception(f"waitforblockheight returned {result!r}") + except JSONRPCException as e: + self.log.debug(f"waitforblockheight raised {e!r}") + assert_equal(e.error['code'], -344) # -344 is RPC timeout + node.wait_until_stopped() + def run_test(self): self.init_pid_test() self.init_stress_test() + self.break_wait_test() if __name__ == '__main__': From c25a5e670b27d3b6eb958ce437dbe89678bd1511 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 1 Oct 2025 13:00:33 -0400 Subject: [PATCH 2/2] init: Signal m_tip_block_cv on Ctrl-C Signal m_tip_block_cv when Ctrl-C is pressed or SIGTERM is received, the same way it is currently signalled when the `stop` RPC is called. This lets RPC calls like `waitforblockheight` and IPC calls like `waitTipChanged` be interrupted, instead of waiting for their original timeouts and delaying shutdown. Historical notes: - The behavior where `stop` RPC signals `m_tip_block_cv`, but CTRL-C does not, has been around since the condition variable was introduced in #30409 (7eccdaf16081d6f624c4dc21df75b0474e049d2b). - The signaling was later moved without changing behavior in #30967 (5ca28ef28bcca1775ff49921fc2528d9439b71ab). This commit moves it again to the Interrupt() function, which is probably the place it should have been added initially, so it works for Ctrl-C shutdowns as well as `stop` shutdowns. - A Qt shutdown bug calling wait methods was fixed previously in #18452 (da73f1513a637a9f347b64de66564d6cdb2541f8), and this change updates that fix to avoid the hang happening again in Qt. --- src/init.cpp | 4 ++-- src/node/interfaces.cpp | 8 +------- test/functional/feature_init.py | 15 +++++---------- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 2f518eba14c..db3d4ab2854 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -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(); InterruptHTTPRPC(); InterruptRPC(); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index fd3fa226cae..27db0467a86 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -132,7 +132,6 @@ public: } void appShutdown() override { - Interrupt(*m_context); Shutdown(*m_context); } void startShutdown() override @@ -141,12 +140,7 @@ public: if (!(Assert(ctx.shutdown_request))()) { LogError("Failed to send shutdown signal\n"); } - - // Stop RPC for clean shutdown if any of waitfor* commands is executed. - if (args().GetBoolArg("-server", false)) { - InterruptRPC(); - StopRPC(); - } + Interrupt(*m_context); } bool shutdownRequested() override { return ShutdownRequested(*Assert(m_context)); }; bool isSettingIgnored(const std::string& name) override diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index d8a773bbe88..0fa99470ddd 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -12,7 +12,6 @@ import signal import subprocess import time -from test_framework.authproxy import JSONRPCException from test_framework.test_framework import BitcoinTestFramework from test_framework.test_node import ( BITCOIN_PID_FILENAME_DEFAULT, @@ -251,9 +250,8 @@ class InitTest(BitcoinTestFramework): terminal. (This can be different than the node shutdown sequence that happens when the stop RPC is sent.) - Currently when the break signal is sent, it does not interrupt the - waitforblockheight RPC call, and the node does not exit until it times - out.""" + The waitforblockheight call should be interrupted and return right away, + and not time out.""" self.log.info("Testing waitforblockheight RPC call followed by break signal") node = self.nodes[0] @@ -287,12 +285,9 @@ class InitTest(BitcoinTestFramework): node.process.send_signal(signal.SIGTERM) node.process.wait() - try: - result = fut.result() - raise Exception(f"waitforblockheight returned {result!r}") - except JSONRPCException as e: - self.log.debug(f"waitforblockheight raised {e!r}") - assert_equal(e.error['code'], -344) # -344 is RPC timeout + result = fut.result() + self.log.debug(f"waitforblockheight returned {result!r}") + assert_equal(result["height"], current_height) node.wait_until_stopped() def run_test(self):