From 814ba36338ae48b6066cd797a4fde84aaeacc4db Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 27 Feb 2024 11:03:43 -0800 Subject: [PATCH] Make sure that current_test_suite and current_test_info are mutex-protected while writing for thread-safety. PiperOrigin-RevId: 610810340 Change-Id: I37f33510373dff04b8e9c9e8a9f32d30fcce46ff --- googletest/include/gtest/gtest.h | 16 ++++++++++ googletest/src/gtest-internal-inl.h | 24 +++++++------- googletest/src/gtest.cc | 49 ++++++++++++++++++----------- 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/googletest/include/gtest/gtest.h b/googletest/include/gtest/gtest.h index de8eceaf..2b70e49c 100644 --- a/googletest/include/gtest/gtest.h +++ b/googletest/include/gtest/gtest.h @@ -1262,6 +1262,20 @@ class GTEST_API_ UnitTest { // total_test_suite_count() - 1. If i is not in that range, returns NULL. TestSuite* GetMutableTestSuite(int i); + // Invokes OsStackTrackGetterInterface::UponLeavingGTest. UponLeavingGTest() + // should be called immediately before Google Test calls user code. It saves + // some information about the current stack that CurrentStackTrace() will use + // to find and hide Google Test stack frames. + void UponLeavingGTest(); + + // Sets the TestSuite object for the test that's currently running. + void set_current_test_suite(TestSuite* a_current_test_suite) + GTEST_LOCK_EXCLUDED_(mutex_); + + // Sets the TestInfo object for the test that's currently running. + void set_current_test_info(TestInfo* a_current_test_info) + GTEST_LOCK_EXCLUDED_(mutex_); + // Accessors for the implementation object. internal::UnitTestImpl* impl() { return impl_; } const internal::UnitTestImpl* impl() const { return impl_; } @@ -1270,6 +1284,8 @@ class GTEST_API_ UnitTest { // members of UnitTest. friend class ScopedTrace; friend class Test; + friend class TestInfo; + friend class TestSuite; friend class internal::AssertHelper; friend class internal::StreamingListenerTest; friend class internal::UnitTestRecordPropertyTestHelper; diff --git a/googletest/src/gtest-internal-inl.h b/googletest/src/gtest-internal-inl.h index 4799a1e7..6dea34f8 100644 --- a/googletest/src/gtest-internal-inl.h +++ b/googletest/src/gtest-internal-inl.h @@ -709,18 +709,6 @@ class GTEST_API_ UnitTestImpl { return type_parameterized_test_registry_; } - // Sets the TestSuite object for the test that's currently running. - void set_current_test_suite(TestSuite* a_current_test_suite) { - current_test_suite_ = a_current_test_suite; - } - - // Sets the TestInfo object for the test that's currently running. If - // current_test_info is NULL, the assertion results will be stored in - // ad_hoc_test_result_. - void set_current_test_info(TestInfo* a_current_test_info) { - current_test_info_ = a_current_test_info; - } - // Registers all parameterized tests defined using TEST_P and // INSTANTIATE_TEST_SUITE_P, creating regular tests for each test/parameter // combination. This method can be called more then once; it has guards @@ -841,6 +829,18 @@ class GTEST_API_ UnitTestImpl { // GTEST_FLAG(catch_exceptions) at the moment it starts. void set_catch_exceptions(bool value) { catch_exceptions_ = value; } + // Sets the TestSuite object for the test that's currently running. + void set_current_test_suite(TestSuite* a_current_test_suite) { + current_test_suite_ = a_current_test_suite; + } + + // Sets the TestInfo object for the test that's currently running. If + // current_test_info is NULL, the assertion results will be stored in + // ad_hoc_test_result_. + void set_current_test_info(TestInfo* a_current_test_info) { + current_test_info_ = a_current_test_info; + } + // The UnitTest object that owns this implementation object. UnitTest* const parent_; diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc index d25f5c43..c5f22bb7 100644 --- a/googletest/src/gtest.cc +++ b/googletest/src/gtest.cc @@ -2836,14 +2836,13 @@ void TestInfo::Run() { } // Tells UnitTest where to store test result. - internal::UnitTestImpl* const impl = internal::GetUnitTestImpl(); - impl->set_current_test_info(this); + UnitTest::GetInstance()->set_current_test_info(this); // Notifies the unit test event listeners that a test is about to start. repeater->OnTestStart(*this); result_.set_start_timestamp(internal::GetTimeInMillis()); internal::Timer timer; - impl->os_stack_trace_getter()->UponLeavingGTest(); + UnitTest::GetInstance()->UponLeavingGTest(); // Creates the test object. Test* const test = internal::HandleExceptionsInMethodIfSupported( @@ -2861,7 +2860,7 @@ void TestInfo::Run() { if (test != nullptr) { // Deletes the test object. - impl->os_stack_trace_getter()->UponLeavingGTest(); + UnitTest::GetInstance()->UponLeavingGTest(); internal::HandleExceptionsInMethodIfSupported( test, &Test::DeleteSelf_, "the test fixture's destructor"); } @@ -2873,15 +2872,14 @@ void TestInfo::Run() { // Tells UnitTest to stop associating assertion results to this // test. - impl->set_current_test_info(nullptr); + UnitTest::GetInstance()->set_current_test_info(nullptr); } // Skip and records a skipped test result for this object. void TestInfo::Skip() { if (!should_run_) return; - internal::UnitTestImpl* const impl = internal::GetUnitTestImpl(); - impl->set_current_test_info(this); + UnitTest::GetInstance()->set_current_test_info(this); TestEventListener* repeater = UnitTest::GetInstance()->listeners().repeater(); @@ -2890,12 +2888,13 @@ void TestInfo::Skip() { const TestPartResult test_part_result = TestPartResult(TestPartResult::kSkip, this->file(), this->line(), ""); - impl->GetTestPartResultReporterForCurrentThread()->ReportTestPartResult( - test_part_result); + internal::GetUnitTestImpl() + ->GetTestPartResultReporterForCurrentThread() + ->ReportTestPartResult(test_part_result); // Notifies the unit test event listener that a test has just finished. repeater->OnTestEnd(*this); - impl->set_current_test_info(nullptr); + UnitTest::GetInstance()->set_current_test_info(nullptr); } // class TestSuite @@ -2991,8 +2990,7 @@ void TestSuite::AddTestInfo(TestInfo* test_info) { void TestSuite::Run() { if (!should_run_) return; - internal::UnitTestImpl* const impl = internal::GetUnitTestImpl(); - impl->set_current_test_suite(this); + UnitTest::GetInstance()->set_current_test_suite(this); TestEventListener* repeater = UnitTest::GetInstance()->listeners().repeater(); @@ -3022,7 +3020,7 @@ void TestSuite::Run() { repeater->OnTestCaseStart(*this); #endif // GTEST_REMOVE_LEGACY_TEST_CASEAPI_ - impl->os_stack_trace_getter()->UponLeavingGTest(); + UnitTest::GetInstance()->UponLeavingGTest(); internal::HandleExceptionsInMethodIfSupported( this, &TestSuite::RunSetUpTestSuite, "SetUpTestSuite()"); @@ -3047,7 +3045,7 @@ void TestSuite::Run() { } elapsed_time_ = timer.Elapsed(); - impl->os_stack_trace_getter()->UponLeavingGTest(); + UnitTest::GetInstance()->UponLeavingGTest(); internal::HandleExceptionsInMethodIfSupported( this, &TestSuite::RunTearDownTestSuite, "TearDownTestSuite()"); @@ -3058,15 +3056,14 @@ void TestSuite::Run() { repeater->OnTestCaseEnd(*this); #endif // GTEST_REMOVE_LEGACY_TEST_CASEAPI_ - impl->set_current_test_suite(nullptr); + UnitTest::GetInstance()->set_current_test_suite(nullptr); } // Skips all tests under this TestSuite. void TestSuite::Skip() { if (!should_run_) return; - internal::UnitTestImpl* const impl = internal::GetUnitTestImpl(); - impl->set_current_test_suite(this); + UnitTest::GetInstance()->set_current_test_suite(this); TestEventListener* repeater = UnitTest::GetInstance()->listeners().repeater(); @@ -3088,7 +3085,7 @@ void TestSuite::Skip() { repeater->OnTestCaseEnd(*this); #endif // GTEST_REMOVE_LEGACY_TEST_CASEAPI_ - impl->set_current_test_suite(nullptr); + UnitTest::GetInstance()->set_current_test_suite(nullptr); } // Clears the results of all tests in this test suite. @@ -5304,6 +5301,22 @@ TestSuite* UnitTest::GetMutableTestSuite(int i) { return impl()->GetMutableSuiteCase(i); } +void UnitTest::UponLeavingGTest() { + impl()->os_stack_trace_getter()->UponLeavingGTest(); +} + +// Sets the TestSuite object for the test that's currently running. +void UnitTest::set_current_test_suite(TestSuite* a_current_test_suite) { + internal::MutexLock lock(&mutex_); + impl_->set_current_test_suite(a_current_test_suite); +} + +// Sets the TestInfo object for the test that's currently running. +void UnitTest::set_current_test_info(TestInfo* a_current_test_info) { + internal::MutexLock lock(&mutex_); + impl_->set_current_test_info(a_current_test_info); +} + // Returns the list of event listeners that can be used to track events // inside Google Test. TestEventListeners& UnitTest::listeners() { return *impl()->listeners(); }