commit 3ab76d945c312f30be8c87f1a1ebbcf562596bda
author Danil Chapovalov <danilchap@webrtc.org> Fri Sep 23 10:39:21 2022
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Sep 23 11:59:57 2022
tree 90821e3752fcb6bd359850b7c0be3feb462ddce2
parent 0deda15c963a72088b7b1271d89e4307fe50dcb7

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 <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38179}

rtc_base/thread.cc

diff --git a/rtc_base/thread.cc b/rtc_base/thread.cc
index fbf4105..18a79bc 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 @@
 namespace rtc {
 namespace {
 
+using ::webrtc::MutexLock;
 using ::webrtc::TimeDelta;
 
 class RTC_SCOPED_LOCKABLE MarkProcessingCritScope {
@@ -426,8 +428,8 @@
     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 @@
   // Signal for the multiplexer to return
 
   {
-    CritScope cs(&crit_);
+    MutexLock lock(&mutex_);
     messages_.push(std::move(task));
   }
   WakeUpSocketServer();
@@ -510,7 +512,7 @@
   int64_t delay_ms = delay.RoundUpTo(webrtc::TimeDelta::Millis(1)).ms<int>();
   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 @@
 }
 
 int Thread::GetDelay() {
-  CritScope cs(&crit_);
+  MutexLock lock(&mutex_);
 
   if (!messages_.empty())
     return 0;
@@ -786,8 +788,10 @@
   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 @@
   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

rtc_base/thread.h

diff --git a/rtc_base/thread.h b/rtc_base/thread.h
index 3fa7533..4b6a797 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 @@
 
   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 @@
 
   // 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 @@
   // Called by the ThreadManager when being unset as the current thread.
   void ClearCurrentTaskQueue();
 
-  std::queue<absl::AnyInvocable<void() &&>> messages_ RTC_GUARDED_BY(crit_);
-  std::priority_queue<DelayedMessage> delayed_messages_ RTC_GUARDED_BY(crit_);
-  uint32_t delayed_next_num_ RTC_GUARDED_BY(crit_);
+  std::queue<absl::AnyInvocable<void() &&>> messages_ RTC_GUARDED_BY(mutex_);
+  std::priority_queue<DelayedMessage> 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<Thread*> 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_;