test: avoid interface_ipc.py race and null pointer dereference #34715

ryanofsky commented at 7:26 PM on March 2, 2026: contributor

Avoid race condition in run_deprecated_mining_test caused by creating and immediately destroying an unused worker thread. This is leading to test failures reported by maflcko in #34711

test: avoid interface_ipc.py race and null pointer dereference

Avoid race condition in run_deprecated_mining_test caused by creating and
immediately destroying an unused worker thread. This leads to test failures
reported by maflcko in #34711

1c1de334e9

DrahtBot added the label Tests on Mar 2, 2026

DrahtBot commented at 7:26 PM on March 2, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	w0xlt, optout21, l0rinc, enirox001

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

achow101 added this to the milestone 31.0 on Mar 2, 2026

fanquake commented at 9:42 AM on March 3, 2026: member

cc @Sjors

w0xlt commented at 6:49 PM on March 3, 2026: contributor

ACK 1c1de334e9c0e3b4c72c3f3fe45b1f4df5ce4502 In run_deprecated_mining_test, ctx was created but never passed to any call, causing the race condition. The only call is init.makeMiningOld2() which takes no arguments.

in test/functional/interface_ipc.py:76 in 1c1de334e9

  70 | @@ -71,7 +71,9 @@ async def async_routine():
  71 |      def run_deprecated_mining_test(self):
  72 |          self.log.info("Running deprecated mining interface test")
  73 |          async def async_routine():
  74 | -            ctx, init = await make_capnp_init_ctx(self)
  75 | +            node = self.nodes[0]
  76 | +            connection = await capnp.AsyncIoStream.create_unix_connection(node.ipc_socket_path)
  77 | +            init = capnp.TwoPartyClient(connection).bootstrap().cast_as(self.capnp_modules['init'].Init)

optout21 commented at 10:57 AM on March 4, 2026:

To reduce code duplication, this could be solved by breaking up make_capnp_init_ctx into two: one to create init only, and the other also the ctx. Counter arguments:

this is a quick test fix
this is the single place where ctx is not needed.

Feel free to resolve this comment if discarded.

optout21 commented at 11:14 AM on March 4, 2026: contributor

utACK 1c1de334e9c0e3b4c72c3f3fe45b1f4df5ce4502

This is a fix for an identified sporadic CI failure, due to a race condition, due to a quick create-and-destroy of a capnp context. The change is test-only and limited. Instead of calling into the make_capnp_init_ctx util, only its needed part is inlined. Verified by code review and unit tests.

l0rinc commented at 12:46 PM on March 4, 2026: contributor

Tested ACK 1c1de334e9c0e3b4c72c3f3fe45b1f4df5ce4502

Added a temporary delay to ProxyServer::makeThread to make the race explicit. It passes after the fix and fails before it.

<details><summary>Widen makeThread race window</summary>

diff --git a/src/ipc/libmultiprocess/src/mp/proxy.cpp b/src/ipc/libmultiprocess/src/mp/proxy.cpp
--- a/src/ipc/libmultiprocess/src/mp/proxy.cpp	(revision 9cad97f6cdf1bd660732cd10e844a6a7e0771ea0)
+++ b/src/ipc/libmultiprocess/src/mp/proxy.cpp	(revision 79e9c9cc5be4932352116e6db0ea84f5f769929c)
@@ -413,6 +413,7 @@
         g_thread_context.thread_name = ThreadName(m_connection.m_loop->m_exe_name) + " (from " + from + ")";
         g_thread_context.waiter = std::make_unique<Waiter>();
         thread_context.set_value(&g_thread_context);
+        usleep(250000); // TODO Reproducer-only delay to widen the race between thread startup and shutdown.
         Lock lock(g_thread_context.waiter->m_mutex);
         // Wait for shutdown signal from ProxyServer<Thread> destructor (signal
         // is just waiter getting set to null.)

</details>

<details><summary>Failure before the fix</summary>

cmake -B build -DCMAKE_BUILD_TYPE=Debug && cmake --build build -j && build/test/functional/test_runner.py interface_ipc.py
...
1/1 - interface_ipc.py failed, Duration: 4 s

stdout:
2026-03-04T12:41:29.808409Z TestFramework (INFO): PRNG seed is: 9184316120798979695
2026-03-04T12:41:29.808817Z TestFramework (INFO): Initializing test directory /var/folders/5t/04gq0pqj5yv4t8cxw51q0s2m0000gn/T/test_runner_₿_🏃_20260304_124129/interface_ipc_0
2026-03-04T12:41:33.104595Z TestFramework (INFO): Running echo test
2026-03-04T12:41:33.361561Z TestFramework (INFO): Running mining test
2026-03-04T12:41:33.619016Z TestFramework (INFO): Running deprecated mining interface test
2026-03-04T12:41:33.675022Z TestFramework (INFO): Stopping nodes
[node 0] Cleaning up ipc directory '/var/folders/5t/04gq0pqj5yv4t8cxw51q0s2m0000gn/T/test-ipc-0e6nefup'


stderr:
Traceback (most recent call last):
  File "/Users/lorinc/IdeaProjects/bitcoin/build/test/functional/interface_ipc.py", line 90, in <module>
    IPCInterfaceTest(__file__).main()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/Users/lorinc/IdeaProjects/bitcoin/test/functional/test_framework/test_framework.py", line 156, in main
    exit_code = self.shutdown()
  File "/Users/lorinc/IdeaProjects/bitcoin/test/functional/test_framework/test_framework.py", line 289, in shutdown
    self.stop_nodes()
    ~~~~~~~~~~~~~~~^^
  File "/Users/lorinc/IdeaProjects/bitcoin/test/functional/test_framework/test_framework.py", line 535, in stop_nodes
    node.wait_until_stopped()
    ~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/Users/lorinc/IdeaProjects/bitcoin/test/functional/test_framework/test_node.py", line 524, in wait_until_stopped
    self.wait_until(lambda: self.is_node_stopped(**kwargs), timeout=timeout)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/lorinc/IdeaProjects/bitcoin/test/functional/test_framework/test_node.py", line 931, in wait_until
    return wait_until_helper_internal(test_function, timeout=timeout, timeout_factor=self.timeout_factor, check_interval=check_interval)
  File "/Users/lorinc/IdeaProjects/bitcoin/test/functional/test_framework/util.py", line 429, in wait_until_helper_internal
    if predicate():
       ~~~~~~~~~^^
  File "/Users/lorinc/IdeaProjects/bitcoin/test/functional/test_framework/test_node.py", line 524, in <lambda>
    self.wait_until(lambda: self.is_node_stopped(**kwargs), timeout=timeout)
                            ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "/Users/lorinc/IdeaProjects/bitcoin/test/functional/test_framework/test_node.py", line 500, in is_node_stopped
    assert return_code in expected_ret_code, self._node_msg(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Node returned unexpected exit code (-11) vs ((0,)) when stopping
[node 0] Cleaning up leftover process



TEST             | STATUS    | DURATION

interface_ipc.py | ✖ Failed  | 4 s
                                                                                                                                                                                                 
ALL              | ✖ Failed  | 4 s (accumulated)                                                                                                                                                 
Runtime: 4 s

</details>

enirox001 commented at 1:10 PM on March 4, 2026: contributor

ACK 1c1de33

Tested locally on Fedora (-DENABLE_IPC=ON).

To verify the race condition, I artificially widened the race window on master by injecting a 100ms std::this_thread::sleep_for inside ProxyServer::makeThread just before the waiter mutex lock. This successfully forced the fatal crash during the test teardown sequence.

Compiling this PR branch with the exact same sabotaged C++ code results in the test passing. Because the test now bypasses make_capnp_init_ctx and connects directly without requesting a remote worker thread, makeThread is never invoked, completely eliminating the premature teardown race condition.

fanquake merged this on Mar 4, 2026

fanquake closed this on Mar 4, 2026