Clarify expectation on GlobalLock

Merge GlobalLock and GlobalLockPod, make member private.
annotate creation of all GlobalLocks with ABSL_CONST_INIT

Bug: None
Change-Id: I29abcc86796ec0e45b15df7d26392309d1bf7324
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/156303
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29447}

media/BUILD.gn

@@ -386,6 +386,7 @@ rtc_static_library("rtc_data") {
     "../rtc_base/third_party/sigslot",
     "../system_wrappers",
     "//third_party/abseil-cpp/absl/algorithm:container",
+    "//third_party/abseil-cpp/absl/base:core_headers",
     "//third_party/abseil-cpp/absl/types:optional",
   ]
 

media/sctp/sctp_transport.cc

@@ -24,6 +24,7 @@ enum PreservedErrno {
 #include <memory>
 
 #include "absl/algorithm/container.h"
+#include "absl/base/attributes.h"
 #include "absl/types/optional.h"
 #include "media/base/codec.h"
 #include "media/base/media_channel.h"
@@ -49,8 +50,8 @@ namespace {
 static constexpr size_t kSctpMtu = 1200;
 
 // Set the initial value of the static SCTP Data Engines reference count.
-int g_usrsctp_usage_count = 0;
-rtc::GlobalLockPod g_usrsctp_lock_;
+ABSL_CONST_INIT int g_usrsctp_usage_count = 0;
+ABSL_CONST_INIT rtc::GlobalLock g_usrsctp_lock_;
 
 // DataMessageType is used for the SCTP "Payload Protocol Identifier", as
 // defined in http://tools.ietf.org/html/rfc4960#section-14.4

pc/BUILD.gn

@@ -124,6 +124,7 @@ rtc_static_library("rtc_pc_base") {
     "../system_wrappers:field_trial",
     "../system_wrappers:metrics",
     "//third_party/abseil-cpp/absl/algorithm:container",
+    "//third_party/abseil-cpp/absl/base:core_headers",
     "//third_party/abseil-cpp/absl/memory",
     "//third_party/abseil-cpp/absl/strings",
     "//third_party/abseil-cpp/absl/types:optional",

pc/srtp_session.cc

@@ -10,6 +10,7 @@
 
 #include "pc/srtp_session.h"
 
+#include "absl/base/attributes.h"
 #include "media/base/rtp_utils.h"
 #include "pc/external_hmac.h"
 #include "rtc_base/critical_section.h"
@@ -362,8 +363,8 @@ bool SrtpSession::UpdateKey(int type,
   return DoSetKey(type, cs, key, len, extension_ids);
 }
 
-int g_libsrtp_usage_count = 0;
-rtc::GlobalLockPod g_libsrtp_lock;
+ABSL_CONST_INIT int g_libsrtp_usage_count = 0;
+ABSL_CONST_INIT rtc::GlobalLock g_libsrtp_lock;
 
 // static
 bool SrtpSession::IncrementLibsrtpUsageCountAndMaybeInit() {

pc/srtp_session.h

@@ -118,7 +118,7 @@ class SrtpSession {
   int rtp_auth_tag_len_ = 0;
   int rtcp_auth_tag_len_ = 0;
   bool inited_ = false;
-  static rtc::GlobalLockPod lock_;
+  static rtc::GlobalLock lock_;
   int last_send_seq_num_ = -1;
   bool external_auth_active_ = false;
   bool external_auth_enabled_ = false;

rtc_base/BUILD.gn

@@ -1197,6 +1197,7 @@ if (rtc_include_tests) {
       "memory:unittests",
       "third_party/base64",
       "third_party/sigslot",
+      "//third_party/abseil-cpp/absl/base:core_headers",
       "//third_party/abseil-cpp/absl/memory",
     ]
   }

rtc_base/critical_section.cc

@@ -216,13 +216,13 @@ CritScope::~CritScope() {
   cs_->Leave();
 }
 
-void GlobalLockPod::Lock() {
+void GlobalLock::Lock() {
 #if !defined(WEBRTC_WIN) && \
     (!defined(WEBRTC_MAC) || RTC_USE_NATIVE_MUTEX_ON_MAC)
   const struct timespec ts_null = {0};
 #endif
 
-  while (AtomicOps::CompareAndSwap(&lock_acquired, 0, 1)) {
+  while (AtomicOps::CompareAndSwap(&lock_acquired_, 0, 1)) {
 #if defined(WEBRTC_WIN)
     ::Sleep(0);
 #elif defined(WEBRTC_MAC) && !RTC_USE_NATIVE_MUTEX_ON_MAC
@@ -233,16 +233,12 @@ void GlobalLockPod::Lock() {
   }
 }
 
-void GlobalLockPod::Unlock() {
-  int old_value = AtomicOps::CompareAndSwap(&lock_acquired, 1, 0);
+void GlobalLock::Unlock() {
+  int old_value = AtomicOps::CompareAndSwap(&lock_acquired_, 1, 0);
   RTC_DCHECK_EQ(1, old_value) << "Unlock called without calling Lock first";
 }
 
-GlobalLock::GlobalLock() {
-  lock_acquired = 0;
-}
-
-GlobalLockScope::GlobalLockScope(GlobalLockPod* lock) : lock_(lock) {
+GlobalLockScope::GlobalLockScope(GlobalLock* lock) : lock_(lock) {
   lock_->Lock();
 }
 

rtc_base/critical_section.h

@@ -94,31 +94,26 @@ class RTC_SCOPED_LOCKABLE CritScope {
   RTC_DISALLOW_COPY_AND_ASSIGN(CritScope);
 };
 
-// A POD lock used to protect global variables. Do NOT use for other purposes.
-// No custom constructor or private data member should be added.
-class RTC_LOCKABLE GlobalLockPod {
+// A lock used to protect global variables. Do NOT use for other purposes.
+class RTC_LOCKABLE GlobalLock {
  public:
-  void Lock() RTC_EXCLUSIVE_LOCK_FUNCTION();
+  constexpr GlobalLock() : lock_acquired_(0) {}
 
+  void Lock() RTC_EXCLUSIVE_LOCK_FUNCTION();
   void Unlock() RTC_UNLOCK_FUNCTION();
 
-  volatile int lock_acquired;
-};
-
-class GlobalLock : public GlobalLockPod {
- public:
-  GlobalLock();
+ private:
+  volatile int lock_acquired_;
 };
 
 // GlobalLockScope, for serializing execution through a scope.
 class RTC_SCOPED_LOCKABLE GlobalLockScope {
  public:
-  explicit GlobalLockScope(GlobalLockPod* lock)
-      RTC_EXCLUSIVE_LOCK_FUNCTION(lock);
+  explicit GlobalLockScope(GlobalLock* lock) RTC_EXCLUSIVE_LOCK_FUNCTION(lock);
   ~GlobalLockScope() RTC_UNLOCK_FUNCTION();
 
  private:
-  GlobalLockPod* const lock_;
+  GlobalLock* const lock_;
   RTC_DISALLOW_COPY_AND_ASSIGN(GlobalLockScope);
 };
 

rtc_base/critical_section_unittest.cc

@@ -15,9 +15,11 @@
 
 #include <memory>
 #include <set>
+#include <type_traits>
 #include <utility>
 #include <vector>
 
+#include "absl/base/attributes.h"
 #include "rtc_base/arraysize.h"
 #include "rtc_base/atomic_ops.h"
 #include "rtc_base/checks.h"
@@ -281,6 +283,13 @@ TEST(AtomicOpsTest, CompareAndSwap) {
   EXPECT_EQ(1, runner.shared_value());
 }
 
+TEST(GlobalLockTest, CanHaveStaticStorageDuration) {
+  static_assert(std::is_trivially_destructible<GlobalLock>::value, "");
+  ABSL_CONST_INIT static GlobalLock global_lock;
+  global_lock.Lock();
+  global_lock.Unlock();
+}
+
 TEST(GlobalLockTest, Basic) {
   // Create and start lots of threads.
   LockRunner<GlobalLock> runner;

rtc_base/system/thread_registry.cc

@@ -30,7 +30,7 @@ struct ThreadData {
 
 // The map of registered threads, and the lock that protects it. We create the
 // map on first use, and never destroy it.
-ABSL_CONST_INIT rtc::GlobalLockPod g_thread_registry_lock = {};
+ABSL_CONST_INIT rtc::GlobalLock g_thread_registry_lock;
 ABSL_CONST_INIT std::map<const ScopedRegisterThreadForDebugging*, ThreadData>*
     g_registered_threads = nullptr;
 

sdk/android/BUILD.gn

@@ -975,6 +975,7 @@ if (current_os == "linux" || is_android) {
       "../../rtc_base:criticalsection",
       "../../rtc_base:logging",
       "../../rtc_base:stringutils",
+      "//third_party/abseil-cpp/absl/base:core_headers",
     ]
   }
 

sdk/android/native_api/stacktrace/stacktrace.cc

@@ -26,6 +26,7 @@
 #undef DS
 #endif
 
+#include "absl/base/attributes.h"
 #include "rtc_base/critical_section.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/strings/string_builder.h"
@@ -91,7 +92,7 @@ struct SignalHandlerOutputState {
 };
 
 // Global lock to ensure only one thread gets interrupted at a time.
-rtc::GlobalLockPod g_signal_handler_lock;
+ABSL_CONST_INIT rtc::GlobalLock g_signal_handler_lock;
 // Argument passed to the ThreadSignalHandler() from the sampling thread to the
 // sampled (stopped) thread. This value is set just before sending signal to the
 // thread and reset when handler is done.