diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 33b074d096..2ab25f269b 100644 --- a/rtc_base/BUILD.gn +++ b/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 = [ diff --git a/rtc_base/synchronization/BUILD.gn b/rtc_base/synchronization/BUILD.gn index ba63f853ff..cbfd6787de 100644 --- a/rtc_base/synchronization/BUILD.gn +++ b/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", ] } diff --git a/rtc_base/synchronization/mutex.h b/rtc_base/synchronization/mutex.h index 7980dae9d7..ea672ecd9c 100644 --- a/rtc_base/synchronization/mutex.h +++ b/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(); } - RTC_WARN_UNUSED_RESULT bool TryLock() RTC_EXCLUSIVE_TRYLOCK_FUNCTION(true) { - return impl_.TryLock(); + void Lock() RTC_EXCLUSIVE_LOCK_FUNCTION() { + rtc::PlatformThreadRef current = CurrentThreadRefAssertingNotBeingHolder(); + 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 holder_ = {0}; }; // MutexLock, for serializing execution through a scope. diff --git a/rtc_base/synchronization/mutex_benchmark.cc b/rtc_base/synchronization/mutex_benchmark.cc index ae7272e8c5..40adca65d8 100644 --- a/rtc_base/synchronization/mutex_benchmark.cc +++ b/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)); } }