From de8a93599a68510fa5d4ba700e5debabfae83ad8 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Wed, 10 Jun 2020 15:40:42 +0200 Subject: [PATCH] Migrate logging to webrtc::Mutex. Bug: webrtc:11567 Change-Id: I5510a29cfa560d20b1f067d772cd06ee4566ea36 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176849 Commit-Queue: Markus Handell Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#31491} --- rtc_base/BUILD.gn | 1 + rtc_base/logging.cc | 21 ++++++++++++--------- rtc_base/synchronization/BUILD.gn | 2 +- rtc_base/synchronization/mutex.h | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 2ab25f269b..c62b3f6afe 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -243,6 +243,7 @@ rtc_library("logging") { ":platform_thread_types", ":stringutils", ":timeutils", + "synchronization:mutex", ] absl_deps = [ "//third_party/abseil-cpp/absl/base:core_headers", diff --git a/rtc_base/logging.cc b/rtc_base/logging.cc index ff7369dd5c..bd2afcc9dd 100644 --- a/rtc_base/logging.cc +++ b/rtc_base/logging.cc @@ -47,6 +47,7 @@ static const int kMaxLogLineSize = 1024 - 60; #include "rtc_base/string_encode.h" #include "rtc_base/string_utils.h" #include "rtc_base/strings/string_builder.h" +#include "rtc_base/synchronization/mutex.h" #include "rtc_base/thread_annotations.h" #include "rtc_base/time_utils.h" @@ -72,7 +73,9 @@ const char* FilenameFromPath(const char* file) { } // Global lock for log subsystem, only needed to serialize access to streams_. -CriticalSection g_log_crit; +// TODO(bugs.webrtc.org/11665): this is not currently constant initialized and +// trivially destructible. +webrtc::Mutex g_log_mutex_; } // namespace ///////////////////////////////////////////////////////////////////////////// @@ -85,7 +88,7 @@ bool LogMessage::log_to_stderr_ = true; // Note: we explicitly do not clean this up, because of the uncertain ordering // of destructors at program exit. Let the person who sets the stream trigger // cleanup by setting to null, or let it leak (safe at program exit). -ABSL_CONST_INIT LogSink* LogMessage::streams_ RTC_GUARDED_BY(g_log_crit) = +ABSL_CONST_INIT LogSink* LogMessage::streams_ RTC_GUARDED_BY(g_log_mutex_) = nullptr; // Boolean options default to false (0) @@ -193,7 +196,7 @@ LogMessage::~LogMessage() { #endif } - CritScope cs(&g_log_crit); + webrtc::MutexLock lock(&g_log_mutex_); for (LogSink* entry = streams_; entry != nullptr; entry = entry->next_) { if (severity_ >= entry->min_severity_) { #if defined(WEBRTC_ANDROID) @@ -242,7 +245,7 @@ void LogMessage::LogTimestamps(bool on) { void LogMessage::LogToDebug(LoggingSeverity min_sev) { g_dbg_sev = min_sev; - CritScope cs(&g_log_crit); + webrtc::MutexLock lock(&g_log_mutex_); UpdateMinLogSeverity(); } @@ -251,7 +254,7 @@ void LogMessage::SetLogToStderr(bool log_to_stderr) { } int LogMessage::GetLogToStream(LogSink* stream) { - CritScope cs(&g_log_crit); + webrtc::MutexLock lock(&g_log_mutex_); LoggingSeverity sev = LS_NONE; for (LogSink* entry = streams_; entry != nullptr; entry = entry->next_) { if (stream == nullptr || stream == entry) { @@ -262,7 +265,7 @@ int LogMessage::GetLogToStream(LogSink* stream) { } void LogMessage::AddLogToStream(LogSink* stream, LoggingSeverity min_sev) { - CritScope cs(&g_log_crit); + webrtc::MutexLock lock(&g_log_mutex_); stream->min_severity_ = min_sev; stream->next_ = streams_; streams_ = stream; @@ -270,7 +273,7 @@ void LogMessage::AddLogToStream(LogSink* stream, LoggingSeverity min_sev) { } void LogMessage::RemoveLogToStream(LogSink* stream) { - CritScope cs(&g_log_crit); + webrtc::MutexLock lock(&g_log_mutex_); for (LogSink** entry = &streams_; *entry != nullptr; entry = &(*entry)->next_) { if (*entry == stream) { @@ -331,7 +334,7 @@ void LogMessage::ConfigureLogging(const char* params) { } void LogMessage::UpdateMinLogSeverity() - RTC_EXCLUSIVE_LOCKS_REQUIRED(g_log_crit) { + RTC_EXCLUSIVE_LOCKS_REQUIRED(g_log_mutex_) { LoggingSeverity min_sev = g_dbg_sev; for (LogSink* entry = streams_; entry != nullptr; entry = entry->next_) { min_sev = std::min(min_sev, entry->min_severity_); @@ -439,7 +442,7 @@ bool LogMessage::IsNoop(LoggingSeverity severity) { // TODO(tommi): We're grabbing this lock for every LogMessage instance that // is going to be logged. This introduces unnecessary synchronization for // a feature that's mostly used for testing. - CritScope cs(&g_log_crit); + webrtc::MutexLock lock(&g_log_mutex_); return streams_ == nullptr; } diff --git a/rtc_base/synchronization/BUILD.gn b/rtc_base/synchronization/BUILD.gn index cbfd6787de..6bc64472fb 100644 --- a/rtc_base/synchronization/BUILD.gn +++ b/rtc_base/synchronization/BUILD.gn @@ -36,7 +36,7 @@ rtc_library("mutex") { ":yield", "..:checks", "..:macromagic", - "..:platform_thread", + "..:platform_thread_types", "../system:unused", ] absl_deps = [ "//third_party/abseil-cpp/absl/base:core_headers" ] diff --git a/rtc_base/synchronization/mutex.h b/rtc_base/synchronization/mutex.h index ea672ecd9c..cc12c7edf0 100644 --- a/rtc_base/synchronization/mutex.h +++ b/rtc_base/synchronization/mutex.h @@ -15,7 +15,7 @@ #include "absl/base/const_init.h" #include "rtc_base/checks.h" -#include "rtc_base/platform_thread.h" +#include "rtc_base/platform_thread_types.h" #include "rtc_base/system/unused.h" #include "rtc_base/thread_annotations.h"