Skip to content

Commit 7970e46

Browse files
committed
race fix: worker thread destroyed before it is initialized
This fixes a race condition in makeThread that can currently trigger segfaults as reported: bitcoin/bitcoin#34711 bitcoin/bitcoin#34756 The problem is a segfault in ProxyServer<ThreadMap>::makeThread calling `Lock lock(g_thread_context.waiter->m_mutex);` that happens because the waiter pointer is null. The waiter pointer can be null if the worker thread is destroyed immediately after it is created, because `~ProxyServer<Thread>` sets it to null. The fix works by moving the lock line above the `thread_context.set_value()` line so the worker thread can't be destroyed before it is fully initialized. A more detailed description of the bug and fix can be found in bitcoin-core#249 (comment) The bug can be reproduced by running the unit test added in the previous commit or by calling makeThread and immediately disconnecting or destroying the returned thread. The bug is not new and has existed since makeThread was implemented, but it was found due to a new functional test in bitcoin core and with antithesis testing (see details in linked issues). The fix was originally posted in bitcoin/bitcoin#34711 (comment)
1 parent ca1ce9c commit 7970e46

1 file changed

Lines changed: 1 addition & 1 deletion

File tree

src/mp/proxy.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,9 +418,9 @@ kj::Promise<void> ProxyServer<ThreadMap>::makeThread(MakeThreadContext context)
418418
std::thread thread([&loop, &thread_context, from]() {
419419
g_thread_context.thread_name = ThreadName(loop.m_exe_name) + " (from " + from + ")";
420420
g_thread_context.waiter = std::make_unique<Waiter>();
421+
Lock lock(g_thread_context.waiter->m_mutex);
421422
thread_context.set_value(&g_thread_context);
422423
if (loop.testing_hook_makethread_created) loop.testing_hook_makethread_created();
423-
Lock lock(g_thread_context.waiter->m_mutex);
424424
// Wait for shutdown signal from ProxyServer<Thread> destructor (signal
425425
// is just waiter getting set to null.)
426426
g_thread_context.waiter->wait(lock, [] { return !g_thread_context.waiter; });

0 commit comments

Comments
 (0)