Mutex: Temporarily add re-entry CHECK.

Bug: webrtc:11567
Change-Id: I8f9f2f8d2f4961fd82ef50de9f4b486056bc7c1e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176701
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31479}

rtc_base/BUILD.gn

@@ -189,6 +189,7 @@ rtc_library("platform_thread") {
     ":rtc_task_queue_libevent",
     ":rtc_task_queue_win",
     ":rtc_task_queue_stdlib",
+    "synchronization:mutex",
     "synchronization:sequence_checker",
   ]
   sources = [

rtc_base/synchronization/BUILD.gn

@@ -36,6 +36,7 @@ rtc_library("mutex") {
     ":yield",
     "..:checks",
     "..:macromagic",
+    "..:platform_thread",
     "../system:unused",
   ]
   absl_deps = [ "//third_party/abseil-cpp/absl/base:core_headers" ]
@@ -115,6 +116,7 @@ if (rtc_include_tests) {
     sources = [ "mutex_benchmark.cc" ]
     deps = [
       ":mutex",
+      "../system:unused",
       "//third_party/google_benchmark",
     ]
   }

rtc_base/synchronization/mutex.h

@@ -15,6 +15,7 @@
 
 #include "absl/base/const_init.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/platform_thread.h"
 #include "rtc_base/system/unused.h"
 #include "rtc_base/thread_annotations.h"
 
@@ -38,14 +39,55 @@ class RTC_LOCKABLE Mutex final {
   Mutex(const Mutex&) = delete;
   Mutex& operator=(const Mutex&) = delete;
 
-  void Lock() RTC_EXCLUSIVE_LOCK_FUNCTION() { impl_.Lock(); }
+  void Lock() RTC_EXCLUSIVE_LOCK_FUNCTION() {
-  RTC_WARN_UNUSED_RESULT bool TryLock() RTC_EXCLUSIVE_TRYLOCK_FUNCTION(true) {
+    rtc::PlatformThreadRef current = CurrentThreadRefAssertingNotBeingHolder();
-    return impl_.TryLock();
+    impl_.Lock();
+    // |holder_| changes from 0 to CurrentThreadRef().
+    holder_.store(current, std::memory_order_relaxed);
+  }
+  RTC_WARN_UNUSED_RESULT bool TryLock() RTC_EXCLUSIVE_TRYLOCK_FUNCTION(true) {
+    rtc::PlatformThreadRef current = CurrentThreadRefAssertingNotBeingHolder();
+    if (impl_.TryLock()) {
+      // |holder_| changes from 0 to CurrentThreadRef().
+      holder_.store(current, std::memory_order_relaxed);
+      return true;
+    }
+    return false;
+  }
+  void Unlock() RTC_UNLOCK_FUNCTION() {
+    // |holder_| changes from CurrentThreadRef() to 0. If something else than
+    // CurrentThreadRef() is stored in |holder_|, the Unlock results in
+    // undefined behavior as mutexes can't be unlocked from another thread than
+    // the one that locked it, or called while not being locked.
+    holder_.store(0, std::memory_order_relaxed);
+    impl_.Unlock();
   }
-  void Unlock() RTC_UNLOCK_FUNCTION() { impl_.Unlock(); }
 
  private:
+  rtc::PlatformThreadRef CurrentThreadRefAssertingNotBeingHolder() {
+    rtc::PlatformThreadRef holder = holder_.load(std::memory_order_relaxed);
+    rtc::PlatformThreadRef current = rtc::CurrentThreadRef();
+    // TODO(bugs.webrtc.org/11567): remove this temporary check after migrating
+    // fully to Mutex.
+    RTC_CHECK_NE(holder, current);
+    return current;
+  }
+
   MutexImpl impl_;
+  // TODO(bugs.webrtc.org/11567): remove |holder_| after migrating fully to
+  // Mutex.
+  // |holder_| contains the PlatformThreadRef of the thread currently holding
+  // the lock, or 0.
+  // Remarks on the used memory orders: the atomic load in
+  // CurrentThreadRefAssertingNotBeingHolder() observes either of two things:
+  // 1. our own previous write to holder_ with our thread ID.
+  // 2. another thread (with ID y) writing y and then 0 from an initial value of
+  // 0. If we're observing case 1, our own stores are obviously ordered before
+  // the load, and hit the CHECK. If we're observing case 2, the value observed
+  // w.r.t |impl_| being locked depends on the memory order. Since we only care
+  // that it's different from CurrentThreadRef()), we use the more performant
+  // option, memory_order_relaxed.
+  std::atomic<rtc::PlatformThreadRef> holder_ = {0};
 };
 
 // MutexLock, for serializing execution through a scope.

rtc_base/synchronization/mutex_benchmark.cc

@@ -10,6 +10,7 @@
 
 #include "benchmark/benchmark.h"
 #include "rtc_base/synchronization/mutex.h"
+#include "rtc_base/system/unused.h"
 
 namespace webrtc {
 
@@ -35,7 +36,8 @@ class PerfTestData {
 
 void BM_LockWithMutex(benchmark::State& state) {
   static PerfTestData test_data;
-  for (auto it = state.begin(); it != state.end(); ++it) {
+  for (auto s : state) {
+    RTC_UNUSED(s);
     benchmark::DoNotOptimize(test_data.AddToCounter(2));
   }
 }