From 4c7bb27a10e575730860494db14f2d1559396f97 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Wed, 15 Jul 2020 13:23:30 +0200 Subject: [PATCH] Remove rtc::GlobalLock. This change migrates a last stray consumer of GlobalLock (SrtpSession) and removes all traces of GlobalLock/GlobalLockScope from WebRTC. Bug: webrtc:11567 Change-Id: I28059f2a10075815a4bdee8c357b9d3b6e50f18b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/179361 Commit-Queue: Tommi Reviewed-by: Tommi Cr-Commit-Position: refs/heads/master@{#31736} --- pc/srtp_session.cc | 9 ++++----- pc/srtp_session.h | 5 +++-- rtc_base/critical_section.cc | 19 ------------------- rtc_base/critical_section.h | 23 ----------------------- rtc_base/critical_section_unittest.cc | 19 ------------------- 5 files changed, 7 insertions(+), 68 deletions(-) diff --git a/pc/srtp_session.cc b/pc/srtp_session.cc index 5ded455ee5..3aa488003f 100644 --- a/pc/srtp_session.cc +++ b/pc/srtp_session.cc @@ -13,7 +13,6 @@ #include "absl/base/attributes.h" #include "media/base/rtp_utils.h" #include "pc/external_hmac.h" -#include "rtc_base/critical_section.h" #include "rtc_base/logging.h" #include "rtc_base/ssl_stream_adapter.h" #include "system_wrappers/include/metrics.h" @@ -364,16 +363,16 @@ bool SrtpSession::UpdateKey(int type, } ABSL_CONST_INIT int g_libsrtp_usage_count = 0; -ABSL_CONST_INIT rtc::GlobalLock g_libsrtp_lock; +ABSL_CONST_INIT webrtc::GlobalMutex g_libsrtp_lock(absl::kConstInit); void ProhibitLibsrtpInitialization() { - rtc::GlobalLockScope ls(&g_libsrtp_lock); + webrtc::GlobalMutexLock ls(&g_libsrtp_lock); ++g_libsrtp_usage_count; } // static bool SrtpSession::IncrementLibsrtpUsageCountAndMaybeInit() { - rtc::GlobalLockScope ls(&g_libsrtp_lock); + webrtc::GlobalMutexLock ls(&g_libsrtp_lock); RTC_DCHECK_GE(g_libsrtp_usage_count, 0); if (g_libsrtp_usage_count == 0) { @@ -402,7 +401,7 @@ bool SrtpSession::IncrementLibsrtpUsageCountAndMaybeInit() { // static void SrtpSession::DecrementLibsrtpUsageCountAndMaybeDeinit() { - rtc::GlobalLockScope ls(&g_libsrtp_lock); + webrtc::GlobalMutexLock ls(&g_libsrtp_lock); RTC_DCHECK_GE(g_libsrtp_usage_count, 1); if (--g_libsrtp_usage_count == 0) { diff --git a/pc/srtp_session.h b/pc/srtp_session.h index e2f1b8d730..84445965b2 100644 --- a/pc/srtp_session.h +++ b/pc/srtp_session.h @@ -14,7 +14,8 @@ #include #include "api/scoped_refptr.h" -#include "rtc_base/critical_section.h" +#include "rtc_base/constructor_magic.h" +#include "rtc_base/synchronization/mutex.h" #include "rtc_base/thread_checker.h" // Forward declaration to avoid pulling in libsrtp headers here @@ -124,7 +125,7 @@ class SrtpSession { int rtp_auth_tag_len_ = 0; int rtcp_auth_tag_len_ = 0; bool inited_ = false; - static rtc::GlobalLock lock_; + static webrtc::GlobalMutex lock_; int last_send_seq_num_ = -1; bool external_auth_active_ = false; bool external_auth_enabled_ = false; diff --git a/rtc_base/critical_section.cc b/rtc_base/critical_section.cc index c6b17ff1b2..1f68299aab 100644 --- a/rtc_base/critical_section.cc +++ b/rtc_base/critical_section.cc @@ -217,23 +217,4 @@ CritScope::~CritScope() { cs_->Leave(); } -void GlobalLock::Lock() { - while (AtomicOps::CompareAndSwap(&lock_acquired_, 0, 1)) { - webrtc::YieldCurrentThread(); - } -} - -void GlobalLock::Unlock() { - int old_value = AtomicOps::CompareAndSwap(&lock_acquired_, 1, 0); - RTC_DCHECK_EQ(1, old_value) << "Unlock called without calling Lock first"; -} - -GlobalLockScope::GlobalLockScope(GlobalLock* lock) : lock_(lock) { - lock_->Lock(); -} - -GlobalLockScope::~GlobalLockScope() { - lock_->Unlock(); -} - } // namespace rtc diff --git a/rtc_base/critical_section.h b/rtc_base/critical_section.h index cf10463bdf..af16861447 100644 --- a/rtc_base/critical_section.h +++ b/rtc_base/critical_section.h @@ -95,29 +95,6 @@ class RTC_SCOPED_LOCKABLE CritScope { RTC_DISALLOW_COPY_AND_ASSIGN(CritScope); }; -// A lock used to protect global variables. Do NOT use for other purposes. -class RTC_LOCKABLE GlobalLock { - public: - constexpr GlobalLock() : lock_acquired_(0) {} - - void Lock() RTC_EXCLUSIVE_LOCK_FUNCTION(); - void Unlock() RTC_UNLOCK_FUNCTION(); - - private: - volatile int lock_acquired_; -}; - -// GlobalLockScope, for serializing execution through a scope. -class RTC_SCOPED_LOCKABLE GlobalLockScope { - public: - explicit GlobalLockScope(GlobalLock* lock) RTC_EXCLUSIVE_LOCK_FUNCTION(lock); - ~GlobalLockScope() RTC_UNLOCK_FUNCTION(); - - private: - GlobalLock* const lock_; - RTC_DISALLOW_COPY_AND_ASSIGN(GlobalLockScope); -}; - } // namespace rtc #endif // RTC_BASE_CRITICAL_SECTION_H_ diff --git a/rtc_base/critical_section_unittest.cc b/rtc_base/critical_section_unittest.cc index 16aefd2740..52d0a8f865 100644 --- a/rtc_base/critical_section_unittest.cc +++ b/rtc_base/critical_section_unittest.cc @@ -282,25 +282,6 @@ TEST(AtomicOpsTest, CompareAndSwap) { EXPECT_EQ(1, runner.shared_value()); } -TEST(GlobalLockTest, CanHaveStaticStorageDuration) { - static_assert(std::is_trivially_destructible::value, ""); - ABSL_CONST_INIT static GlobalLock global_lock; - global_lock.Lock(); - global_lock.Unlock(); -} - -TEST(GlobalLockTest, Basic) { - // Create and start lots of threads. - LockRunner runner; - std::vector> threads; - StartThreads(&threads, &runner); - runner.SetExpectedThreadCount(kNumThreads); - - // Release the hounds! - EXPECT_TRUE(runner.Run()); - EXPECT_EQ(0, runner.shared_value()); -} - TEST(CriticalSectionTest, Basic) { // Create and start lots of threads. LockRunner runner;