Add method Mutex::AssertHeld

Acts as a compile time annotation, with corresponding run-time check
only when DCHECKs are enabled, and built using absl or pthreads mutexes.

Bug: None
Change-Id: Ie044c1ea1e576df71d634301f7df9d75cdf10b1b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/226328
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34555}

modules/audio_processing/audio_processing_impl.h

@@ -143,6 +143,11 @@ class AudioProcessingImpl : public AudioProcessing {
   // Overridden in a mock.
   virtual void InitializeLocked()
       RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_render_, mutex_capture_);
+  void AssertLockedForTest()
+      RTC_ASSERT_EXCLUSIVE_LOCK(mutex_render_, mutex_capture_) {
+    mutex_render_.AssertHeld();
+    mutex_capture_.AssertHeld();
+  }
 
  private:
   // TODO(peah): These friend classes should be removed as soon as the new

modules/audio_processing/audio_processing_impl_unittest.cc

@@ -39,7 +39,8 @@ class MockInitialize : public AudioProcessingImpl {
       : AudioProcessingImpl(config) {}
 
   MOCK_METHOD(void, InitializeLocked, (), (override));
-  void RealInitializeLocked() RTC_NO_THREAD_SAFETY_ANALYSIS {
+  void RealInitializeLocked() {
+    AssertLockedForTest();
     AudioProcessingImpl::InitializeLocked();
   }
 

rtc_base/synchronization/BUILD.gn

@@ -37,6 +37,7 @@ rtc_library("mutex") {
     "..:checks",
     "..:macromagic",
     "..:platform_thread_types",
+    "../system:no_unique_address",
   ]
   absl_deps = [ "//third_party/abseil-cpp/absl/base:core_headers" ]
   if (rtc_use_absl_mutex) {

rtc_base/synchronization/mutex.h

@@ -44,6 +44,10 @@ class RTC_LOCKABLE Mutex final {
   ABSL_MUST_USE_RESULT bool TryLock() RTC_EXCLUSIVE_TRYLOCK_FUNCTION(true) {
     return impl_.TryLock();
   }
+  // Return immediately if this thread holds the mutex, or RTC_DCHECK_IS_ON==0.
+  // Otherwise, may report an error (typically by crashing with a diagnostic),
+  // or may return immediately.
+  void AssertHeld() const RTC_ASSERT_EXCLUSIVE_LOCK() { impl_.AssertHeld(); }
   void Unlock() RTC_UNLOCK_FUNCTION() {
     impl_.Unlock();
   }

rtc_base/synchronization/mutex_abseil.h

@@ -27,6 +27,11 @@ class RTC_LOCKABLE MutexImpl final {
   ABSL_MUST_USE_RESULT bool TryLock() RTC_EXCLUSIVE_TRYLOCK_FUNCTION(true) {
     return mutex_.TryLock();
   }
+  void AssertHeld() const RTC_ASSERT_EXCLUSIVE_LOCK() {
+#if RTC_DCHECK_IS_ON
+    mutex_.AssertHeld();
+#endif
+  }
   void Unlock() RTC_UNLOCK_FUNCTION() { mutex_.Unlock(); }
 
  private:

rtc_base/synchronization/mutex_critical_section.h

@@ -41,6 +41,7 @@ class RTC_LOCKABLE MutexImpl final {
   ABSL_MUST_USE_RESULT bool TryLock() RTC_EXCLUSIVE_TRYLOCK_FUNCTION(true) {
     return TryEnterCriticalSection(&critical_section_) != FALSE;
   }
+  void AssertHeld() const RTC_ASSERT_EXCLUSIVE_LOCK() {}
   void Unlock() RTC_UNLOCK_FUNCTION() {
     LeaveCriticalSection(&critical_section_);
   }

rtc_base/synchronization/mutex_pthread.h

@@ -19,6 +19,7 @@
 #endif
 
 #include "absl/base/attributes.h"
+#include "rtc_base/system/no_unique_address.h"
 #include "rtc_base/thread_annotations.h"
 
 namespace webrtc {
@@ -39,14 +40,60 @@ class RTC_LOCKABLE MutexImpl final {
   MutexImpl& operator=(const MutexImpl&) = delete;
   ~MutexImpl() { pthread_mutex_destroy(&mutex_); }
 
-  void Lock() RTC_EXCLUSIVE_LOCK_FUNCTION() { pthread_mutex_lock(&mutex_); }
-  ABSL_MUST_USE_RESULT bool TryLock() RTC_EXCLUSIVE_TRYLOCK_FUNCTION(true) {
-    return pthread_mutex_trylock(&mutex_) == 0;
+  void Lock() RTC_EXCLUSIVE_LOCK_FUNCTION() {
+    pthread_mutex_lock(&mutex_);
+    owner_.SetOwner();
+  }
+  ABSL_MUST_USE_RESULT bool TryLock() RTC_EXCLUSIVE_TRYLOCK_FUNCTION(true) {
+    if (pthread_mutex_trylock(&mutex_) != 0) {
+      return false;
+    }
+    owner_.SetOwner();
+    return true;
+  }
+  void AssertHeld() const RTC_ASSERT_EXCLUSIVE_LOCK() { owner_.AssertOwned(); }
+  void Unlock() RTC_UNLOCK_FUNCTION() {
+    owner_.ClearOwner();
+    pthread_mutex_unlock(&mutex_);
   }
-  void Unlock() RTC_UNLOCK_FUNCTION() { pthread_mutex_unlock(&mutex_); }
 
  private:
+  class OwnerRecord {
+   public:
+#if !RTC_DCHECK_IS_ON
+    void SetOwner() {}
+    void ClearOwner() {}
+    void AssertOwned() const {}
+#else
+    void SetOwner() {
+      latest_owner_ = pthread_self();
+      is_owned_ = true;
+    }
+    void ClearOwner() { is_owned_ = false; }
+    void AssertOwned() const {
+      RTC_CHECK(is_owned_);
+      RTC_CHECK(pthread_equal(latest_owner_, pthread_self()));
+    }
+
+   private:
+    // Use two separate primitive types, rather than absl::optional, since the
+    // data race described below might invalidate absl::optional invariants.
+    bool is_owned_ = false;
+    pthread_t latest_owner_ = pthread_self();
+#endif
+  };
+
   pthread_mutex_t mutex_;
+  // This record is modified only with the mutex held, and hence, calls to
+  // AssertHeld where mutex is held are race-free and will always succeed.
+  //
+  // The failure case is more subtle: If AssertHeld is called from some thread
+  // not holding the mutex, and RTC_DCHECK_IS_ON==1, we have a data race. It is
+  // highly likely that the calling thread will see `is_owned_` false or
+  // `latest_owner_` different from itself, and crash. But it may fail to crash,
+  // and invoke some other undefined behavior (still, this race can happen only
+  // when RTC_DCHECK_IS_ON==1).
+  RTC_NO_UNIQUE_ADDRESS OwnerRecord owner_;
 };
 
 }  // namespace webrtc

rtc_base/thread_annotations.h

@@ -88,6 +88,9 @@
 #define RTC_UNLOCK_FUNCTION(...) \
   RTC_THREAD_ANNOTATION_ATTRIBUTE__(unlock_function(__VA_ARGS__))
 
+#define RTC_ASSERT_EXCLUSIVE_LOCK(...) \
+  RTC_THREAD_ANNOTATION_ATTRIBUTE__(assert_exclusive_lock(__VA_ARGS__))
+
 // An escape hatch for thread safety analysis to ignore the annotated function.
 #define RTC_NO_THREAD_SAFETY_ANALYSIS \
   RTC_THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis)