diff --git a/rtc_base/DEPS b/rtc_base/DEPS index c9f7dc5898..3fdc4bc10e 100644 --- a/rtc_base/DEPS +++ b/rtc_base/DEPS @@ -12,4 +12,7 @@ specific_include_rules = { "gunit\.h": [ "+testing/base/public/gunit.h" ], + "logging\.cc": [ + "+absl/synchronization" + ], } diff --git a/rtc_base/logging.cc b/rtc_base/logging.cc index 13a5f02597..a333d83970 100644 --- a/rtc_base/logging.cc +++ b/rtc_base/logging.cc @@ -51,6 +51,17 @@ static const int kMaxLogLineSize = 1024 - 60; #include "rtc_base/thread_annotations.h" #include "rtc_base/time_utils.h" +#if defined(WEBRTC_RACE_CHECK_MUTEX) +#if defined(WEBRTC_ABSL_MUTEX) +#error Please only define one of WEBRTC_RACE_CHECK_MUTEX and WEBRTC_ABSL_MUTEX. +#endif +#include "absl/base/const_init.h" +#include "absl/synchronization/mutex.h" // nogncheck +using LoggingMutexLock = ::absl::MutexLock; +#else +using LoggingMutexLock = ::webrtc::MutexLock; +#endif // if defined(WEBRTC_RACE_CHECK_MUTEX) + namespace rtc { namespace { // By default, release builds don't log, debug builds at info level @@ -75,7 +86,15 @@ const char* FilenameFromPath(const char* file) { // Global lock for log subsystem, only needed to serialize access to streams_. // TODO(bugs.webrtc.org/11665): this is not currently constant initialized and // trivially destructible. +#if defined(WEBRTC_RACE_CHECK_MUTEX) +// When WEBRTC_RACE_CHECK_MUTEX is defined, even though WebRTC objects are +// invoked serially, the logging is static, invoked concurrently and hence needs +// protection. +absl::Mutex g_log_mutex_(absl::kConstInit); +#else webrtc::Mutex g_log_mutex_; +#endif + } // namespace ///////////////////////////////////////////////////////////////////////////// @@ -201,7 +220,7 @@ LogMessage::~LogMessage() { #endif } - webrtc::MutexLock lock(&g_log_mutex_); + LoggingMutexLock lock(&g_log_mutex_); for (LogSink* entry = streams_; entry != nullptr; entry = entry->next_) { if (severity_ >= entry->min_severity_) { #if defined(WEBRTC_ANDROID) @@ -250,7 +269,7 @@ void LogMessage::LogTimestamps(bool on) { void LogMessage::LogToDebug(LoggingSeverity min_sev) { g_dbg_sev = min_sev; - webrtc::MutexLock lock(&g_log_mutex_); + LoggingMutexLock lock(&g_log_mutex_); UpdateMinLogSeverity(); } @@ -259,7 +278,7 @@ void LogMessage::SetLogToStderr(bool log_to_stderr) { } int LogMessage::GetLogToStream(LogSink* stream) { - webrtc::MutexLock lock(&g_log_mutex_); + LoggingMutexLock lock(&g_log_mutex_); LoggingSeverity sev = LS_NONE; for (LogSink* entry = streams_; entry != nullptr; entry = entry->next_) { if (stream == nullptr || stream == entry) { @@ -270,7 +289,7 @@ int LogMessage::GetLogToStream(LogSink* stream) { } void LogMessage::AddLogToStream(LogSink* stream, LoggingSeverity min_sev) { - webrtc::MutexLock lock(&g_log_mutex_); + LoggingMutexLock lock(&g_log_mutex_); stream->min_severity_ = min_sev; stream->next_ = streams_; streams_ = stream; @@ -279,7 +298,7 @@ void LogMessage::AddLogToStream(LogSink* stream, LoggingSeverity min_sev) { } void LogMessage::RemoveLogToStream(LogSink* stream) { - webrtc::MutexLock lock(&g_log_mutex_); + LoggingMutexLock lock(&g_log_mutex_); for (LogSink** entry = &streams_; *entry != nullptr; entry = &(*entry)->next_) { if (*entry == stream) { diff --git a/rtc_base/synchronization/BUILD.gn b/rtc_base/synchronization/BUILD.gn index 73ff667246..3cddc55c72 100644 --- a/rtc_base/synchronization/BUILD.gn +++ b/rtc_base/synchronization/BUILD.gn @@ -27,6 +27,7 @@ rtc_library("mutex") { "mutex.h", "mutex_critical_section.h", "mutex_pthread.h", + "mutex_race_check.h", ] if (rtc_use_absl_mutex) { sources += [ "mutex_abseil.h" ] @@ -37,6 +38,7 @@ rtc_library("mutex") { "..:checks", "..:macromagic", "..:platform_thread_types", + "../system:unused", ] absl_deps = [ "//third_party/abseil-cpp/absl/base:core_headers" ] if (rtc_use_absl_mutex) { diff --git a/rtc_base/synchronization/mutex.h b/rtc_base/synchronization/mutex.h index 0023d90ef5..e1512e96cc 100644 --- a/rtc_base/synchronization/mutex.h +++ b/rtc_base/synchronization/mutex.h @@ -18,7 +18,12 @@ #include "rtc_base/checks.h" #include "rtc_base/thread_annotations.h" -#if defined(WEBRTC_ABSL_MUTEX) +#if defined(WEBRTC_RACE_CHECK_MUTEX) +// To use the race check mutex, define WEBRTC_RACE_CHECK_MUTEX globally. This +// also adds a dependency to absl::Mutex from logging.cc due to concurrent +// invocation of the static logging system. +#include "rtc_base/synchronization/mutex_race_check.h" +#elif defined(WEBRTC_ABSL_MUTEX) #include "rtc_base/synchronization/mutex_abseil.h" // nogncheck #elif defined(WEBRTC_WIN) #include "rtc_base/synchronization/mutex_critical_section.h" diff --git a/rtc_base/synchronization/mutex_race_check.h b/rtc_base/synchronization/mutex_race_check.h new file mode 100644 index 0000000000..7a79d8a965 --- /dev/null +++ b/rtc_base/synchronization/mutex_race_check.h @@ -0,0 +1,64 @@ +/* + * Copyright 2020 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef RTC_BASE_SYNCHRONIZATION_MUTEX_RACE_CHECK_H_ +#define RTC_BASE_SYNCHRONIZATION_MUTEX_RACE_CHECK_H_ + +#include + +#include "rtc_base/checks.h" +#include "rtc_base/system/unused.h" +#include "rtc_base/thread_annotations.h" + +namespace webrtc { + +// This implementation class is useful when a consuming project can guarantee +// that all WebRTC invocation is happening serially. Additionally, the consuming +// project cannot use WebRTC code that spawn threads or task queues. +// +// The class internally check fails on Lock() if it finds the consumer actually +// invokes WebRTC concurrently. +// +// To use the race check mutex, define WEBRTC_RACE_CHECK_MUTEX globally. This +// also adds a dependency to absl::Mutex from logging.cc because even though +// objects are invoked serially, the logging is static and invoked concurrently +// and hence needs protection. +class RTC_LOCKABLE MutexImpl final { + public: + MutexImpl() = default; + MutexImpl(const MutexImpl&) = delete; + MutexImpl& operator=(const MutexImpl&) = delete; + + void Lock() RTC_EXCLUSIVE_LOCK_FUNCTION() { + bool was_free = free_.exchange(false, std::memory_order_acquire); + RTC_CHECK(was_free) + << "WEBRTC_RACE_CHECK_MUTEX: mutex locked concurrently."; + } + RTC_WARN_UNUSED_RESULT bool TryLock() RTC_EXCLUSIVE_TRYLOCK_FUNCTION(true) { + bool was_free = free_.exchange(false, std::memory_order_acquire); + return was_free; + } + void Unlock() RTC_UNLOCK_FUNCTION() { + free_.store(true, std::memory_order_release); + } + + private: + // Release-acquire ordering is used. + // - In the Lock methods we're guaranteeing that reads and writes happening + // after the (Try)Lock don't appear to have happened before the Lock (acquire + // ordering). + // - In the Unlock method we're guaranteeing that reads and writes happening + // before the Unlock don't appear to happen after it (release ordering). + std::atomic free_{true}; +}; + +} // namespace webrtc + +#endif // RTC_BASE_SYNCHRONIZATION_MUTEX_RACE_CHECK_H_