From 1b96fa13f549387b7549cc89e1a785cf143a1a50 Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Tue, 11 Nov 2025 18:56:19 -0800 Subject: [PATCH] Switch to referenceful lock holder for Abseil compatibility PiperOrigin-RevId: 831156684 Change-Id: I8a8b017ec2fc318a65f57e04428c030c707ee682 --- .../include/gmock/gmock-spec-builders.h | 4 +-- googlemock/src/gmock-internal-utils.cc | 2 +- googlemock/src/gmock-spec-builders.cc | 28 +++++++++---------- .../include/gtest/internal/gtest-port.h | 16 +++++------ googletest/src/gtest-port.cc | 6 ++-- googletest/src/gtest.cc | 26 ++++++++--------- googletest/test/googletest-port-test.cc | 14 ++++++---- 7 files changed, 48 insertions(+), 48 deletions(-) diff --git a/googlemock/include/gmock/gmock-spec-builders.h b/googlemock/include/gmock/gmock-spec-builders.h index e861817f4..f1c979d6f 100644 --- a/googlemock/include/gmock/gmock-spec-builders.h +++ b/googlemock/include/gmock/gmock-spec-builders.h @@ -1467,7 +1467,7 @@ class FunctionMocker final : public UntypedFunctionMockerBase { // function have been satisfied. If not, it will report Google Test // non-fatal failures for the violations. ~FunctionMocker() override GTEST_LOCK_EXCLUDED_(g_gmock_mutex) { - MutexLock l(&g_gmock_mutex); + MutexLock l(g_gmock_mutex); VerifyAndClearExpectationsLocked(); Mock::UnregisterLocked(this); ClearDefaultActionsLocked(); @@ -1646,7 +1646,7 @@ class FunctionMocker final : public UntypedFunctionMockerBase { GTEST_LOCK_EXCLUDED_(g_gmock_mutex) { const ArgumentTuple& args = *static_cast(untyped_args); - MutexLock l(&g_gmock_mutex); + MutexLock l(g_gmock_mutex); TypedExpectation* exp = this->FindMatchingExpectationLocked(args); if (exp == nullptr) { // A match wasn't found. this->FormatUnexpectedCallMessageLocked(args, what, why); diff --git a/googlemock/src/gmock-internal-utils.cc b/googlemock/src/gmock-internal-utils.cc index 96c7e306e..ceee329a8 100644 --- a/googlemock/src/gmock-internal-utils.cc +++ b/googlemock/src/gmock-internal-utils.cc @@ -156,7 +156,7 @@ GTEST_API_ void Log(LogSeverity severity, const std::string& message, if (!LogIsVisible(severity)) return; // Ensures that logs from different threads don't interleave. - MutexLock l(&g_log_mutex); + MutexLock l(g_log_mutex); if (severity == kWarning) { // Prints a GMOCK WARNING marker to make the warnings easily searchable. diff --git a/googlemock/src/gmock-spec-builders.cc b/googlemock/src/gmock-spec-builders.cc index 603ad7aa0..88e0c0200 100644 --- a/googlemock/src/gmock-spec-builders.cc +++ b/googlemock/src/gmock-spec-builders.cc @@ -212,7 +212,7 @@ void ExpectationBase::CheckActionCountIfNotDone() const GTEST_LOCK_EXCLUDED_(mutex_) { bool should_check = false; { - MutexLock l(&mutex_); + MutexLock l(mutex_); if (!action_count_checked_) { action_count_checked_ = true; should_check = true; @@ -318,7 +318,7 @@ UntypedFunctionMockerBase::~UntypedFunctionMockerBase() = default; void UntypedFunctionMockerBase::RegisterOwner(const void* mock_obj) GTEST_LOCK_EXCLUDED_(g_gmock_mutex) { { - MutexLock l(&g_gmock_mutex); + MutexLock l(g_gmock_mutex); mock_obj_ = mock_obj; } Mock::Register(mock_obj, this); @@ -332,7 +332,7 @@ void UntypedFunctionMockerBase::SetOwnerAndName(const void* mock_obj, GTEST_LOCK_EXCLUDED_(g_gmock_mutex) { // We protect name_ under g_gmock_mutex in case this mock function // is called from two threads concurrently. - MutexLock l(&g_gmock_mutex); + MutexLock l(g_gmock_mutex); mock_obj_ = mock_obj; name_ = name; } @@ -345,7 +345,7 @@ const void* UntypedFunctionMockerBase::MockObject() const { // We protect mock_obj_ under g_gmock_mutex in case this mock // function is called from two threads concurrently. - MutexLock l(&g_gmock_mutex); + MutexLock l(g_gmock_mutex); Assert(mock_obj_ != nullptr, __FILE__, __LINE__, "MockObject() must not be called before RegisterOwner() or " "SetOwnerAndName() has been called."); @@ -362,7 +362,7 @@ const char* UntypedFunctionMockerBase::Name() const { // We protect name_ under g_gmock_mutex in case this mock // function is called from two threads concurrently. - MutexLock l(&g_gmock_mutex); + MutexLock l(g_gmock_mutex); Assert(name_ != nullptr, __FILE__, __LINE__, "Name() must not be called before SetOwnerAndName() has " "been called."); @@ -490,7 +490,7 @@ class MockObjectRegistry { // failure, unless the user explicitly asked us to ignore it. ~MockObjectRegistry() { if (!GMOCK_FLAG_GET(catch_leaked_mocks)) return; - internal::MutexLock l(&internal::g_gmock_mutex); + internal::MutexLock l(internal::g_gmock_mutex); int leaked_count = 0; for (StateMap::const_iterator it = states_.begin(); it != states_.end(); @@ -559,7 +559,7 @@ UninterestingCallReactionMap() { void SetReactionOnUninterestingCalls(uintptr_t mock_obj, internal::CallReaction reaction) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex) { - internal::MutexLock l(&internal::g_gmock_mutex); + internal::MutexLock l(internal::g_gmock_mutex); UninterestingCallReactionMap()[mock_obj] = reaction; } @@ -590,7 +590,7 @@ void Mock::FailUninterestingCalls(uintptr_t mock_obj) // entry in the call-reaction table should be removed. void Mock::UnregisterCallReaction(uintptr_t mock_obj) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex) { - internal::MutexLock l(&internal::g_gmock_mutex); + internal::MutexLock l(internal::g_gmock_mutex); UninterestingCallReactionMap().erase(static_cast(mock_obj)); } @@ -598,7 +598,7 @@ void Mock::UnregisterCallReaction(uintptr_t mock_obj) // made on the given mock object. internal::CallReaction Mock::GetReactionOnUninterestingCalls( const void* mock_obj) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex) { - internal::MutexLock l(&internal::g_gmock_mutex); + internal::MutexLock l(internal::g_gmock_mutex); return (UninterestingCallReactionMap().count( reinterpret_cast(mock_obj)) == 0) ? internal::intToCallReaction( @@ -611,7 +611,7 @@ internal::CallReaction Mock::GetReactionOnUninterestingCalls( // objects. void Mock::AllowLeak(const void* mock_obj) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex) { - internal::MutexLock l(&internal::g_gmock_mutex); + internal::MutexLock l(internal::g_gmock_mutex); g_mock_object_registry.states()[mock_obj].leakable = true; } @@ -620,7 +620,7 @@ void Mock::AllowLeak(const void* mock_obj) // Test non-fatal failures and returns false. bool Mock::VerifyAndClearExpectations(void* mock_obj) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex) { - internal::MutexLock l(&internal::g_gmock_mutex); + internal::MutexLock l(internal::g_gmock_mutex); return VerifyAndClearExpectationsLocked(mock_obj); } @@ -629,7 +629,7 @@ bool Mock::VerifyAndClearExpectations(void* mock_obj) // verification was successful. bool Mock::VerifyAndClear(void* mock_obj) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex) { - internal::MutexLock l(&internal::g_gmock_mutex); + internal::MutexLock l(internal::g_gmock_mutex); ClearDefaultActionsLocked(mock_obj); return VerifyAndClearExpectationsLocked(mock_obj); } @@ -679,7 +679,7 @@ bool Mock::IsStrict(void* mock_obj) void Mock::Register(const void* mock_obj, internal::UntypedFunctionMockerBase* mocker) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex) { - internal::MutexLock l(&internal::g_gmock_mutex); + internal::MutexLock l(internal::g_gmock_mutex); g_mock_object_registry.states()[mock_obj].function_mockers.insert(mocker); } @@ -689,7 +689,7 @@ void Mock::Register(const void* mock_obj, void Mock::RegisterUseByOnCallOrExpectCall(const void* mock_obj, const char* file, int line) GTEST_LOCK_EXCLUDED_(internal::g_gmock_mutex) { - internal::MutexLock l(&internal::g_gmock_mutex); + internal::MutexLock l(internal::g_gmock_mutex); MockObjectState& state = g_mock_object_registry.states()[mock_obj]; if (state.first_used_file == nullptr) { state.first_used_file = file; diff --git a/googletest/include/gtest/internal/gtest-port.h b/googletest/include/gtest/internal/gtest-port.h index f810d0644..d433f0545 100644 --- a/googletest/include/gtest/internal/gtest-port.h +++ b/googletest/include/gtest/internal/gtest-port.h @@ -1424,12 +1424,11 @@ class GTEST_API_ Mutex { // "MutexLock l(&mu)". Hence the typedef trick below. class GTestMutexLock { public: - explicit GTestMutexLock(Mutex* mutex) : mutex_(mutex) { mutex_->lock(); } - - ~GTestMutexLock() { mutex_->unlock(); } + explicit GTestMutexLock(Mutex& mutex) : mutex_(mutex) { mutex_.lock(); } + ~GTestMutexLock() { mutex_.unlock(); } private: - Mutex* const mutex_; + Mutex& mutex_; GTestMutexLock(const GTestMutexLock&) = delete; GTestMutexLock& operator=(const GTestMutexLock&) = delete; @@ -1716,12 +1715,11 @@ class Mutex : public MutexBase { // "MutexLock l(&mu)". Hence the typedef trick below. class GTestMutexLock { public: - explicit GTestMutexLock(MutexBase* mutex) : mutex_(mutex) { mutex_->lock(); } - - ~GTestMutexLock() { mutex_->unlock(); } + explicit GTestMutexLock(MutexBase& mutex) : mutex_(mutex) { mutex_.lock(); } + ~GTestMutexLock() { mutex_.unlock(); } private: - MutexBase* const mutex_; + MutexBase& mutex_; GTestMutexLock(const GTestMutexLock&) = delete; GTestMutexLock& operator=(const GTestMutexLock&) = delete; @@ -1881,7 +1879,7 @@ class Mutex { // "MutexLock l(&mu)". Hence the typedef trick below. class GTestMutexLock { public: - explicit GTestMutexLock(Mutex*) {} // NOLINT + explicit GTestMutexLock(Mutex&) {} // NOLINT }; typedef GTestMutexLock MutexLock; diff --git a/googletest/src/gtest-port.cc b/googletest/src/gtest-port.cc index 490dbb579..d50d07cdb 100644 --- a/googletest/src/gtest-port.cc +++ b/googletest/src/gtest-port.cc @@ -499,7 +499,7 @@ class ThreadLocalRegistryImpl { MemoryIsNotDeallocated memory_is_not_deallocated; #endif // _MSC_VER DWORD current_thread = ::GetCurrentThreadId(); - MutexLock lock(&mutex_); + MutexLock lock(mutex_); ThreadIdToThreadLocals* const thread_to_thread_locals = GetThreadLocalsMapLocked(); ThreadIdToThreadLocals::iterator thread_local_pos = @@ -532,7 +532,7 @@ class ThreadLocalRegistryImpl { // Clean up the ThreadLocalValues data structure while holding the lock, but // defer the destruction of the ThreadLocalValueHolderBases. { - MutexLock lock(&mutex_); + MutexLock lock(mutex_); ThreadIdToThreadLocals* const thread_to_thread_locals = GetThreadLocalsMapLocked(); for (ThreadIdToThreadLocals::iterator it = @@ -559,7 +559,7 @@ class ThreadLocalRegistryImpl { // Clean up the ThreadIdToThreadLocals data structure while holding the // lock, but defer the destruction of the ThreadLocalValueHolderBases. { - MutexLock lock(&mutex_); + MutexLock lock(mutex_); ThreadIdToThreadLocals* const thread_to_thread_locals = GetThreadLocalsMapLocked(); ThreadIdToThreadLocals::iterator thread_local_pos = diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc index 76b49e903..80a7edad9 100644 --- a/googletest/src/gtest.cc +++ b/googletest/src/gtest.cc @@ -1086,14 +1086,14 @@ void DefaultPerThreadTestPartResultReporter::ReportTestPartResult( // Returns the global test part result reporter. TestPartResultReporterInterface* UnitTestImpl::GetGlobalTestPartResultReporter() { - internal::MutexLock lock(&global_test_part_result_reporter_mutex_); + internal::MutexLock lock(global_test_part_result_reporter_mutex_); return global_test_part_result_reporter_; } // Sets the global test part result reporter. void UnitTestImpl::SetGlobalTestPartResultReporter( TestPartResultReporterInterface* reporter) { - internal::MutexLock lock(&global_test_part_result_reporter_mutex_); + internal::MutexLock lock(global_test_part_result_reporter_mutex_); global_test_part_result_reporter_ = reporter; } @@ -2347,7 +2347,7 @@ void TestResult::RecordProperty(const std::string& xml_element, if (!ValidateTestProperty(xml_element, test_property)) { return; } - internal::MutexLock lock(&test_properties_mutex_); + internal::MutexLock lock(test_properties_mutex_); const std::vector::iterator property_with_matching_key = std::find_if(test_properties_.begin(), test_properties_.end(), internal::TestPropertyKeyIs(test_property.key())); @@ -5088,7 +5088,7 @@ std::string OsStackTraceGetter::CurrentStackTrace(int max_depth, int skip_count) void* caller_frame = nullptr; { - MutexLock lock(&mutex_); + MutexLock lock(mutex_); caller_frame = caller_frame_; } @@ -5127,7 +5127,7 @@ void OsStackTraceGetter::UponLeavingGTest() GTEST_LOCK_EXCLUDED_(mutex_) { caller_frame = nullptr; } - MutexLock lock(&mutex_); + MutexLock lock(mutex_); caller_frame_ = caller_frame; #endif // GTEST_HAS_ABSL } @@ -5390,13 +5390,13 @@ void UnitTest::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_); + 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_); + internal::MutexLock lock(mutex_); impl_->set_current_test_info(a_current_test_info); } @@ -5435,7 +5435,7 @@ void UnitTest::AddTestPartResult(TestPartResult::Type result_type, Message msg; msg << message; - internal::MutexLock lock(&mutex_); + internal::MutexLock lock(mutex_); if (!impl_->gtest_trace_stack().empty()) { msg << "\n" << GTEST_NAME_ << " trace:"; @@ -5618,7 +5618,7 @@ const char* UnitTest::original_working_dir() const { // or NULL if no test is running. const TestSuite* UnitTest::current_test_suite() const GTEST_LOCK_EXCLUDED_(mutex_) { - internal::MutexLock lock(&mutex_); + internal::MutexLock lock(mutex_); return impl_->current_test_suite(); } @@ -5626,7 +5626,7 @@ const TestSuite* UnitTest::current_test_suite() const #ifndef GTEST_REMOVE_LEGACY_TEST_CASEAPI_ const TestCase* UnitTest::current_test_case() const GTEST_LOCK_EXCLUDED_(mutex_) { - internal::MutexLock lock(&mutex_); + internal::MutexLock lock(mutex_); return impl_->current_test_suite(); } #endif @@ -5635,7 +5635,7 @@ const TestCase* UnitTest::current_test_case() const // or NULL if no test is running. const TestInfo* UnitTest::current_test_info() const GTEST_LOCK_EXCLUDED_(mutex_) { - internal::MutexLock lock(&mutex_); + internal::MutexLock lock(mutex_); return impl_->current_test_info(); } @@ -5659,13 +5659,13 @@ UnitTest::~UnitTest() { delete impl_; } // Google Test trace stack. void UnitTest::PushGTestTrace(const internal::TraceInfo& trace) GTEST_LOCK_EXCLUDED_(mutex_) { - internal::MutexLock lock(&mutex_); + internal::MutexLock lock(mutex_); impl_->gtest_trace_stack().push_back(trace); } // Pops a trace from the per-thread Google Test trace stack. void UnitTest::PopGTestTrace() GTEST_LOCK_EXCLUDED_(mutex_) { - internal::MutexLock lock(&mutex_); + internal::MutexLock lock(mutex_); impl_->gtest_trace_stack().pop_back(); } diff --git a/googletest/test/googletest-port-test.cc b/googletest/test/googletest-port-test.cc index 975b2fc33..24fcd9d93 100644 --- a/googletest/test/googletest-port-test.cc +++ b/googletest/test/googletest-port-test.cc @@ -308,7 +308,7 @@ TEST(GetThreadCountTest, ReturnsCorrectValue) { internal::Mutex mutex; { - internal::MutexLock lock(&mutex); + internal::MutexLock lock(mutex); pthread_attr_t attr; ASSERT_EQ(0, pthread_attr_init(&attr)); ASSERT_EQ(0, pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE)); @@ -1028,7 +1028,9 @@ TEST(MutexDeathTest, AssertHeldShouldAssertWhenNotLocked) { EXPECT_DEATH_IF_SUPPORTED( { Mutex m; - { MutexLock lock(&m); } + { + MutexLock lock(m); + } m.AssertHeld(); }, "thread .*hold"); @@ -1036,13 +1038,13 @@ TEST(MutexDeathTest, AssertHeldShouldAssertWhenNotLocked) { TEST(MutexTest, AssertHeldShouldNotAssertWhenLocked) { Mutex m; - MutexLock lock(&m); + MutexLock lock(m); m.AssertHeld(); } class AtomicCounterWithMutex { public: - explicit AtomicCounterWithMutex(Mutex* mutex) + explicit AtomicCounterWithMutex(Mutex& mutex) : value_(0), mutex_(mutex), random_(42) {} void Increment() { @@ -1083,7 +1085,7 @@ class AtomicCounterWithMutex { private: volatile int value_; - Mutex* const mutex_; // Protects value_. + Mutex& mutex_; // Protects value_. Random random_; }; @@ -1094,7 +1096,7 @@ void CountingThreadFunc(pair param) { // Tests that the mutex only lets one thread at a time to lock it. TEST(MutexTest, OnlyOneThreadCanLockAtATime) { Mutex mutex; - AtomicCounterWithMutex locked_counter(&mutex); + AtomicCounterWithMutex locked_counter(mutex); typedef ThreadWithParam > ThreadType; const int kCycleCount = 20;