From 3ab76d945c312f30be8c87f1a1ebbcf562596bda Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 23 Sep 2022 12:39:21 +0200 Subject: [PATCH] Use non-recursive mutex in rtc::Thread With rtc::Thread::Clear removed, there are no longer calls to external code while holding the mutex and thus it doesn't need to be recursive. Bug: webrtc:11567 Change-Id: If350684be0bfcbc33806019bd986138905aec66d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276542 Reviewed-by: Tomas Gunnarsson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#38179} --- rtc_base/thread.cc | 26 +++++++++++++++----------- rtc_base/thread.h | 13 +++++++------ 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/rtc_base/thread.cc b/rtc_base/thread.cc index fbf4105a8a..18a79bc518 100644 --- a/rtc_base/thread.cc +++ b/rtc_base/thread.cc @@ -41,6 +41,7 @@ #include "rtc_base/internal/default_socket_server.h" #include "rtc_base/logging.h" #include "rtc_base/null_socket_server.h" +#include "rtc_base/synchronization/mutex.h" #include "rtc_base/time_utils.h" #include "rtc_base/trace_event.h" @@ -74,6 +75,7 @@ class ScopedAutoReleasePool { namespace rtc { namespace { +using ::webrtc::MutexLock; using ::webrtc::TimeDelta; class RTC_SCOPED_LOCKABLE MarkProcessingCritScope { @@ -426,8 +428,8 @@ absl::AnyInvocable Thread::Get(int cmsWait) { int64_t cmsDelayNext = kForever; { // All queue operations need to be locked, but nothing else in this loop - // can happen inside the crit. - CritScope cs(&crit_); + // can happen while holding the `mutex_`. + MutexLock lock(&mutex_); // Check for delayed messages that have been triggered and calculate the // next trigger time. while (!delayed_messages_.empty()) { @@ -491,7 +493,7 @@ void Thread::PostTask(absl::AnyInvocable task) { // Signal for the multiplexer to return { - CritScope cs(&crit_); + MutexLock lock(&mutex_); messages_.push(std::move(task)); } WakeUpSocketServer(); @@ -510,7 +512,7 @@ void Thread::PostDelayedHighPrecisionTask(absl::AnyInvocable task, int64_t delay_ms = delay.RoundUpTo(webrtc::TimeDelta::Millis(1)).ms(); int64_t run_time_ms = TimeAfter(delay_ms); { - CritScope cs(&crit_); + MutexLock lock(&mutex_); delayed_messages_.push({.delay_ms = delay_ms, .run_time_ms = run_time_ms, .message_number = delayed_next_num_, @@ -525,7 +527,7 @@ void Thread::PostDelayedHighPrecisionTask(absl::AnyInvocable task, } int Thread::GetDelay() { - CritScope cs(&crit_); + MutexLock lock(&mutex_); if (!messages_.empty()) return 0; @@ -786,8 +788,10 @@ void Thread::BlockingCall(rtc::FunctionView functor) { absl::Cleanup cleanup = [this, &ready, current_thread, done = done_event.get()] { if (current_thread) { - CritScope cs(&crit_); - ready = true; + { + MutexLock lock(&mutex_); + ready = true; + } current_thread->socketserver()->WakeUp(); } else { done->Set(); @@ -796,14 +800,14 @@ void Thread::BlockingCall(rtc::FunctionView functor) { PostTask([functor, cleanup = std::move(cleanup)] { functor(); }); if (current_thread) { bool waited = false; - crit_.Enter(); + mutex_.Lock(); while (!ready) { - crit_.Leave(); + mutex_.Unlock(); current_thread->socketserver()->Wait(SocketServer::kForever, false); waited = true; - crit_.Enter(); + mutex_.Lock(); } - crit_.Leave(); + mutex_.Unlock(); // Our Wait loop above may have consumed some WakeUp events for this // Thread, that weren't relevant to this Send. Losing these WakeUps can diff --git a/rtc_base/thread.h b/rtc_base/thread.h index 3fa75330a5..4b6a7971a7 100644 --- a/rtc_base/thread.h +++ b/rtc_base/thread.h @@ -38,6 +38,7 @@ #include "rtc_base/location.h" #include "rtc_base/platform_thread_types.h" #include "rtc_base/socket_server.h" +#include "rtc_base/synchronization/mutex.h" #include "rtc_base/system/rtc_export.h" #include "rtc_base/thread_annotations.h" @@ -270,7 +271,7 @@ class RTC_LOCKABLE RTC_EXPORT Thread : public webrtc::TaskQueueBase { bool empty() const { return size() == 0u; } size_t size() const { - CritScope cs(&crit_); + webrtc::MutexLock lock(&mutex_); return messages_.size() + delayed_messages_.size(); } @@ -429,7 +430,7 @@ class RTC_LOCKABLE RTC_EXPORT Thread : public webrtc::TaskQueueBase { // Perform cleanup; subclasses must call this from the destructor, // and are not expected to actually hold the lock. - void DoDestroy() RTC_EXCLUSIVE_LOCKS_REQUIRED(&crit_); + void DoDestroy() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); void WakeUpSocketServer(); @@ -482,16 +483,16 @@ class RTC_LOCKABLE RTC_EXPORT Thread : public webrtc::TaskQueueBase { // Called by the ThreadManager when being unset as the current thread. void ClearCurrentTaskQueue(); - std::queue> messages_ RTC_GUARDED_BY(crit_); - std::priority_queue delayed_messages_ RTC_GUARDED_BY(crit_); - uint32_t delayed_next_num_ RTC_GUARDED_BY(crit_); + std::queue> messages_ RTC_GUARDED_BY(mutex_); + std::priority_queue delayed_messages_ RTC_GUARDED_BY(mutex_); + uint32_t delayed_next_num_ RTC_GUARDED_BY(mutex_); #if RTC_DCHECK_IS_ON uint32_t blocking_call_count_ RTC_GUARDED_BY(this) = 0; uint32_t could_be_blocking_call_count_ RTC_GUARDED_BY(this) = 0; std::vector allowed_threads_ RTC_GUARDED_BY(this); bool invoke_policy_enabled_ RTC_GUARDED_BY(this) = false; #endif - RecursiveCriticalSection crit_; + mutable webrtc::Mutex mutex_; bool fInitialized_; bool fDestroyed_;