From b86bf840d1d562d12efd4e0790ce14455f5fa271 Mon Sep 17 00:00:00 2001 From: Tanzinul Islam Date: Sun, 3 Sep 2023 15:39:19 +0100 Subject: [PATCH] Count threads after thread-creation while still holding mutex lock The `Mutex` is locked with the `MutexLock` before spawning the thread, so that the thread is prevented from completing (by being blocked on `Mutex`) before the new thread count is obtained. However, the existing bug (introduced in 22e6055) releases `Mutex` before obtaining the new thread count, which allows the thread to run to completion in the meantime. Also, since the `(thread_count_after_create != starting_count + 1)` condition (line 328) skips the remainder of the `for`-loop body on every iteration, `thread_count_after_join` stays uninitialized. I believe this is why [this test failed][1] on the macOS CI with this trace: ``` [----------] 1 test from GetThreadCountTest [ RUN ] GetThreadCountTest.ReturnsCorrectValue googletest/test/googletest-port-test.cc:350: Failure Expected equality of these values: thread_count_after_create Which is: 1 starting_count + 1 Which is: 2 googletest/test/googletest-port-test.cc:351: Failure Expected equality of these values: thread_count_after_join Which is: 140493185949400 starting_count Which is: 1 [ FAILED ] GetThreadCountTest.ReturnsCorrectValue (2 ms) [----------] 1 test from GetThreadCountTest (2 ms total) ``` [1]: https://github.com/google/googletest/actions/runs/6064919420/job/16453860690?pr=3049 --- googletest/test/googletest-port-test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/googletest/test/googletest-port-test.cc b/googletest/test/googletest-port-test.cc index e0793bad..8d210260 100644 --- a/googletest/test/googletest-port-test.cc +++ b/googletest/test/googletest-port-test.cc @@ -296,7 +296,7 @@ void* ThreadFunc(void* data) { TEST(GetThreadCountTest, ReturnsCorrectValue) { size_t starting_count; size_t thread_count_after_create; - size_t thread_count_after_join; + size_t thread_count_after_join = 0; // We can't guarantee that no other thread was created or destroyed between // any two calls to GetThreadCount(). We make multiple attempts, hoping that @@ -316,9 +316,9 @@ TEST(GetThreadCountTest, ReturnsCorrectValue) { const int status = pthread_create(&thread_id, &attr, &ThreadFunc, &mutex); ASSERT_EQ(0, pthread_attr_destroy(&attr)); ASSERT_EQ(0, status); - } - thread_count_after_create = GetThreadCount(); + thread_count_after_create = GetThreadCount(); + } void* dummy; ASSERT_EQ(0, pthread_join(thread_id, &dummy));