Skip to content

Commit d3a29ff

Browse files
derekmaurocopybara-github
authored andcommitted
Fix a race condition between the Watcher thread and the main thread
during program exit A race condition exist between the Watcher thread and main(). A case was found where the Watcher thread does not get execution time before the main function returns and calls atexit(). At that point the Watcher thread started runing tls_init() code while the main thread was shutting down. This resulted in rare crashes and deadlocks. Fixes #4493 Closes #4494 PiperOrigin-RevId: 621619768 Change-Id: I66f00d8f0f3c37f9937c6d13890f7fa10038256d
1 parent ec7b386 commit d3a29ff

File tree

1 file changed

+25
-10
lines changed

1 file changed

+25
-10
lines changed

googletest/src/gtest-port.cc

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -587,24 +587,31 @@ class ThreadLocalRegistryImpl {
587587
// thread's ID.
588588
typedef std::map<DWORD, ThreadLocalValues> ThreadIdToThreadLocals;
589589

590-
// Holds the thread id and thread handle that we pass from
591-
// StartWatcherThreadFor to WatcherThreadFunc.
592-
typedef std::pair<DWORD, HANDLE> ThreadIdAndHandle;
590+
struct WatcherThreadParams {
591+
DWORD thread_id;
592+
HANDLE handle;
593+
Notification has_initialized;
594+
};
593595

594596
static void StartWatcherThreadFor(DWORD thread_id) {
595597
// The returned handle will be kept in thread_map and closed by
596598
// watcher_thread in WatcherThreadFunc.
597599
HANDLE thread =
598600
::OpenThread(SYNCHRONIZE | THREAD_QUERY_INFORMATION, FALSE, thread_id);
599601
GTEST_CHECK_(thread != nullptr);
602+
603+
WatcherThreadParams* watcher_thread_params = new WatcherThreadParams;
604+
watcher_thread_params->thread_id = thread_id;
605+
watcher_thread_params->handle = thread;
606+
600607
// We need to pass a valid thread ID pointer into CreateThread for it
601608
// to work correctly under Win98.
602609
DWORD watcher_thread_id;
603610
HANDLE watcher_thread = ::CreateThread(
604611
nullptr, // Default security.
605612
0, // Default stack size
606613
&ThreadLocalRegistryImpl::WatcherThreadFunc,
607-
reinterpret_cast<LPVOID>(new ThreadIdAndHandle(thread_id, thread)),
614+
reinterpret_cast<LPVOID>(watcher_thread_params),
608615
CREATE_SUSPENDED, &watcher_thread_id);
609616
GTEST_CHECK_(watcher_thread != nullptr)
610617
<< "CreateThread failed with error " << ::GetLastError() << ".";
@@ -614,17 +621,25 @@ class ThreadLocalRegistryImpl {
614621
::GetThreadPriority(::GetCurrentThread()));
615622
::ResumeThread(watcher_thread);
616623
::CloseHandle(watcher_thread);
624+
625+
// Wait for the watcher thread to start to avoid race conditions.
626+
// One specific race condition that can happen is that we have returned
627+
// from main and have started to tear down, the newly spawned watcher
628+
// thread may access already-freed variables, like global shared_ptrs.
629+
watcher_thread_params->has_initialized.WaitForNotification();
617630
}
618631

619632
// Monitors exit from a given thread and notifies those
620633
// ThreadIdToThreadLocals about thread termination.
621634
static DWORD WINAPI WatcherThreadFunc(LPVOID param) {
622-
const ThreadIdAndHandle* tah =
623-
reinterpret_cast<const ThreadIdAndHandle*>(param);
624-
GTEST_CHECK_(::WaitForSingleObject(tah->second, INFINITE) == WAIT_OBJECT_0);
625-
OnThreadExit(tah->first);
626-
::CloseHandle(tah->second);
627-
delete tah;
635+
WatcherThreadParams* watcher_thread_params =
636+
reinterpret_cast<WatcherThreadParams*>(param);
637+
watcher_thread_params->has_initialized.Notify();
638+
GTEST_CHECK_(::WaitForSingleObject(watcher_thread_params->handle,
639+
INFINITE) == WAIT_OBJECT_0);
640+
OnThreadExit(watcher_thread_params->thread_id);
641+
::CloseHandle(watcher_thread_params->handle);
642+
delete watcher_thread_params;
628643
return 0;
629644
}
630645

0 commit comments

Comments
 (0)