diff --git a/internal/platform/implementation/apple/BUILD b/internal/platform/implementation/apple/BUILD index 182787fe..50b68b35 100644 --- a/internal/platform/implementation/apple/BUILD +++ b/internal/platform/implementation/apple/BUILD @@ -347,6 +347,7 @@ cc_test( ":Platform_cc", "//internal/platform/implementation/g3:crypto", "@com_github_protobuf_matchers//protobuf-matchers", + "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/synchronization", "@com_google_absl//absl/time", "@com_google_googletest//:gtest_main", diff --git a/internal/platform/implementation/apple/mutex.h b/internal/platform/implementation/apple/mutex.h index 3a7890b2..210aa883 100644 --- a/internal/platform/implementation/apple/mutex.h +++ b/internal/platform/implementation/apple/mutex.h @@ -15,6 +15,7 @@ #ifndef PLATFORM_IMPL_APPLE_MUTEX_H_ #define PLATFORM_IMPL_APPLE_MUTEX_H_ +#include "absl/base/thread_annotations.h" #include "absl/synchronization/mutex.h" #include "internal/platform/implementation/mutex.h" @@ -36,6 +37,10 @@ class ABSL_LOCKABLE Mutex : public api::Mutex { } void Unlock() ABSL_UNLOCK_FUNCTION() override { mutex_.Unlock(); } + void AssertHeld() const ABSL_ASSERT_EXCLUSIVE_LOCK() override { + mutex_.AssertHeld(); + } + private: friend class ConditionVariable; absl::Mutex mutex_; @@ -65,6 +70,12 @@ class ABSL_LOCKABLE RecursiveMutex : public api::Mutex { } } + void AssertHeld() const ABSL_ASSERT_EXCLUSIVE_LOCK() override { +#ifndef NDEBUG + assert(thread_id_.load(std::memory_order_acquire) == ThreadId()); +#endif + } + private: static inline intptr_t ThreadId() { ABSL_CONST_INIT thread_local int per_thread = 0; diff --git a/internal/platform/implementation/apple/mutex_test.cc b/internal/platform/implementation/apple/mutex_test.cc index 4a9bded4..4ebadce0 100644 --- a/internal/platform/implementation/apple/mutex_test.cc +++ b/internal/platform/implementation/apple/mutex_test.cc @@ -15,9 +15,8 @@ #include "internal/platform/implementation/apple/mutex.h" #include "gtest/gtest.h" -#include "absl/synchronization/mutex.h" +#include "absl/base/thread_annotations.h" #include "absl/synchronization/notification.h" -#include "absl/time/clock.h" #include "absl/time/time.h" #include "thread/fiber/fiber.h" @@ -85,6 +84,36 @@ TEST(MutexTest, RecursiveLockingForNestedWorks) { f.Join(); } +TEST(MutexTest, AssertHeld) { + Mutex mutex; + mutex.Lock(); + mutex.AssertHeld(); + mutex.Unlock(); +} + +TEST(MutexTest, RecursiveAssertHeld) ABSL_NO_THREAD_SAFETY_ANALYSIS { + RecursiveMutex mutex; + mutex.Lock(); + mutex.AssertHeld(); + mutex.Lock(); + mutex.AssertHeld(); + mutex.Unlock(); + mutex.AssertHeld(); + mutex.Unlock(); +} + +#if !defined(NDEBUG) && defined(GTEST_HAS_DEATH_TEST) +TEST(MutexDeathTest, MutexAssertHeldWithoutLock) { + Mutex mutex; + EXPECT_DEATH(mutex.AssertHeld(), ""); +} + +TEST(MutexDeathTest, RecursiveMutexAssertHeldWithoutLock) { + RecursiveMutex mutex; + EXPECT_DEATH(mutex.AssertHeld(), ""); +} +#endif // !defined(NDEBUG) && defined(GTEST_HAS_DEATH_TEST) + } // namespace } // namespace apple } // namespace nearby diff --git a/internal/platform/implementation/g3/mutex.h b/internal/platform/implementation/g3/mutex.h index c808731f..0b9e9b2f 100644 --- a/internal/platform/implementation/g3/mutex.h +++ b/internal/platform/implementation/g3/mutex.h @@ -15,6 +15,7 @@ #ifndef PLATFORM_IMPL_G3_MUTEX_H_ #define PLATFORM_IMPL_G3_MUTEX_H_ +#include "absl/base/thread_annotations.h" #include "absl/synchronization/mutex.h" #include "internal/platform/implementation/mutex.h" #include "internal/platform/implementation/shared/posix_mutex.h" @@ -35,8 +36,13 @@ class ABSL_LOCKABLE Mutex : public api::Mutex { mutex_.Lock(); if (!check_) mutex_.ForgetDeadlockInfo(); } + void Unlock() ABSL_UNLOCK_FUNCTION() override { mutex_.Unlock(); } + void AssertHeld() const ABSL_ASSERT_EXCLUSIVE_LOCK() override { + mutex_.AssertHeld(); + } + private: friend class ConditionVariable; absl::Mutex mutex_; diff --git a/internal/platform/implementation/mutex.h b/internal/platform/implementation/mutex.h index 622f1056..d226cc64 100644 --- a/internal/platform/implementation/mutex.h +++ b/internal/platform/implementation/mutex.h @@ -45,6 +45,8 @@ class ABSL_LOCKABLE Mutex { virtual void Lock() ABSL_EXCLUSIVE_LOCK_FUNCTION() = 0; virtual void Unlock() ABSL_UNLOCK_FUNCTION() = 0; + // Assert that this mutex is held by the calling thread. + virtual void AssertHeld() const ABSL_ASSERT_EXCLUSIVE_LOCK() {} }; } // namespace api diff --git a/internal/platform/implementation/shared/posix_mutex.h b/internal/platform/implementation/shared/posix_mutex.h index b1d15d0a..183a14d8 100644 --- a/internal/platform/implementation/shared/posix_mutex.h +++ b/internal/platform/implementation/shared/posix_mutex.h @@ -29,6 +29,7 @@ class ABSL_LOCKABLE Mutex : public api::Mutex { void Lock() ABSL_EXCLUSIVE_LOCK_FUNCTION() override; void Unlock() ABSL_UNLOCK_FUNCTION() override; + void AssertHeld() const ABSL_ASSERT_EXCLUSIVE_LOCK() override {} private: friend class ConditionVariable; diff --git a/internal/platform/implementation/windows/mutex.h b/internal/platform/implementation/windows/mutex.h index 006293e9..44488a23 100644 --- a/internal/platform/implementation/windows/mutex.h +++ b/internal/platform/implementation/windows/mutex.h @@ -52,6 +52,12 @@ class ABSL_LOCKABLE Mutex : public api::Mutex { } } + void AssertHeld() const ABSL_ASSERT_EXCLUSIVE_LOCK() override { + if (mode_ == Mode::kRegular || mode_ == Mode::kRegularNoCheck) { + mutex_.AssertHeld(); + } + } + absl::Mutex& GetMutex() { return mutex_; } std::recursive_mutex& GetRecursiveMutex() { return recursive_mutex_; } diff --git a/internal/platform/mutex.h b/internal/platform/mutex.h index 6ce81a69..f1c5808f 100644 --- a/internal/platform/mutex.h +++ b/internal/platform/mutex.h @@ -41,6 +41,7 @@ class ABSL_LOCKABLE Mutex final { void Lock() ABSL_EXCLUSIVE_LOCK_FUNCTION() { impl_->Lock(); } void Unlock() ABSL_UNLOCK_FUNCTION() { impl_->Unlock(); } + void AssertHeld() const ABSL_ASSERT_EXCLUSIVE_LOCK() { impl_->AssertHeld(); } private: friend class ConditionVariable; @@ -65,6 +66,7 @@ class ABSL_LOCKABLE RecursiveMutex final { void Lock() ABSL_EXCLUSIVE_LOCK_FUNCTION() { impl_->Lock(); } void Unlock() ABSL_UNLOCK_FUNCTION() { impl_->Unlock(); } + void AssertHeld() const ABSL_ASSERT_EXCLUSIVE_LOCK() { impl_->AssertHeld(); } private: friend class MutexLock; diff --git a/internal/platform/mutex_test.cc b/internal/platform/mutex_test.cc index 0e22212c..8e70e65d 100644 --- a/internal/platform/mutex_test.cc +++ b/internal/platform/mutex_test.cc @@ -14,11 +14,13 @@ #include "internal/platform/mutex.h" +#include + #include "gtest/gtest.h" +#include "absl/base/thread_annotations.h" #include "absl/synchronization/mutex.h" #include "absl/time/clock.h" #include "absl/time/time.h" -#include "internal/platform/condition_variable.h" #include "internal/platform/single_thread_executor.h" namespace nearby { @@ -111,5 +113,38 @@ TEST_F(MutexTest, DoubleLockIsNotDeadlock) { VerifyStepReached(2); } +TEST_F(MutexTest, AssertHeld) { + Mutex mutex; + mutex.Lock(); + mutex.AssertHeld(); + mutex.Unlock(); +} + +TEST_F(MutexTest, RecursiveAssertHeld) ABSL_NO_THREAD_SAFETY_ANALYSIS { + RecursiveMutex mutex; + mutex.Lock(); + mutex.AssertHeld(); + mutex.Lock(); + mutex.AssertHeld(); + mutex.Unlock(); + mutex.AssertHeld(); + mutex.Unlock(); +} + +#ifndef NDEBUG +using MutexDeathTest = MutexTest; +// Mutex death test may fail on some platforms. +TEST_F(MutexDeathTest, DISABLED_MutexAssertHeldWithoutLock) { + Mutex mutex; + EXPECT_DEATH(mutex.AssertHeld(), ""); +} + +// RecursiveMutex death test may fail on some platforms. +TEST_F(MutexDeathTest, DISABLED_RecursiveMutexAssertHeldWithoutLock) { + RecursiveMutex mutex; + EXPECT_DEATH(mutex.AssertHeld(), ""); +} +#endif + } // namespace } // namespace nearby