From b0423b0d4591f08a9c16f39c44946d4eb92fff1c Mon Sep 17 00:00:00 2001 From: Sergiu Deitsch Date: Fri, 6 Oct 2023 00:50:09 +0200 Subject: [PATCH] feat: use standard mutexes --- CMakeLists.txt | 13 +- bazel/glog.bzl | 3 +- src/base/mutex.h | 352 ---------------------------------------- src/config.h.cmake.in | 3 - src/logging.cc | 76 +++++---- src/logging_unittest.cc | 27 +-- src/utilities.h | 1 - src/vlog_is_on.cc | 11 +- 8 files changed, 72 insertions(+), 414 deletions(-) delete mode 100644 src/base/mutex.h diff --git a/CMakeLists.txt b/CMakeLists.txt index f96625ced..16614335b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -167,11 +167,6 @@ if (Threads_FOUND) set (CMAKE_REQUIRED_LIBRARIES Threads::Threads) endif (Threads_FOUND) -check_cxx_symbol_exists (pthread_rwlock_destroy pthread.h HAVE_RWLOCK_DESTROY) -check_cxx_symbol_exists (pthread_rwlock_init pthread.h HAVE_RWLOCK_INIT) -check_cxx_symbol_exists (pthread_rwlock_rdlock pthread.h HAVE_RWLOCK_RDLOCK) -check_cxx_symbol_exists (pthread_rwlock_unlock pthread.h HAVE_RWLOCK_UNLOCK) -check_cxx_symbol_exists (pthread_rwlock_wrlock pthread.h HAVE_RWLOCK_WRLOCK) check_cxx_symbol_exists (pthread_threadid_np pthread.h HAVE_PTHREAD_THREADID_NP) cmake_pop_check_state () @@ -391,7 +386,6 @@ set (GLOG_SRCS ${GLOG_PUBLIC_H} src/base/commandlineflags.h src/base/googleinit.h - src/base/mutex.h src/demangle.cc src/demangle.h src/logging.cc @@ -440,6 +434,13 @@ set (glog_libraries_options_for_static_linking) # CMake always uses the generated export header target_compile_definitions (glog PUBLIC GLOG_USE_GLOG_EXPORT) +if (WIN32) + # Do not define min and max as macros + target_compile_definitions (glog PRIVATE NOMINMAX) + # Exclude unnecessary funcitonality + target_compile_definitions (glog PRIVATE WIN32_LEAN_AND_MEAN) +endif (WIN32) + if (HAVE_LIB_GFLAGS) target_compile_definitions (glog PUBLIC GLOG_USE_GFLAGS) endif (HAVE_LIB_GFLAGS) diff --git a/bazel/glog.bzl b/bazel/glog.bzl index a5642aff9..13c7ae74e 100644 --- a/bazel/glog.bzl +++ b/bazel/glog.bzl @@ -59,7 +59,7 @@ def glog_library(with_gflags = 1, **kwargs): "-Wno-unused-function", "-Wno-unused-local-typedefs", "-Wno-unused-variable", - # Allows src/base/mutex.h to include pthread.h. + # Allows to include pthread.h. "-DHAVE_PTHREAD", # Allows src/logging.cc to determine the host name. "-DHAVE_SYS_UTSNAME_H", @@ -154,7 +154,6 @@ def glog_library(with_gflags = 1, **kwargs): name = "shared_headers", srcs = [ "src/base/commandlineflags.h", - "src/base/mutex.h", "src/stacktrace.h", "src/utilities.h", ] diff --git a/src/base/mutex.h b/src/base/mutex.h deleted file mode 100644 index bb7424635..000000000 --- a/src/base/mutex.h +++ /dev/null @@ -1,352 +0,0 @@ -// Copyright (c) 2007, Google Inc. -// All rights reserved. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following disclaimer -// in the documentation and/or other materials provided with the -// distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived from -// this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -// -// --- -// Author: Craig Silverstein. -// -// A simple mutex wrapper, supporting locks and read-write locks. -// You should assume the locks are *not* re-entrant. -// -// To use: you should define the following macros in your configure.ac: -// ACX_PTHREAD -// AC_RWLOCK -// The latter is defined in ../autoconf. -// -// This class is meant to be internal-only and should be wrapped by an -// internal namespace. Before you use this module, please give the -// name of your internal namespace for this module. Or, if you want -// to expose it, you'll want to move it to the Google namespace. We -// cannot put this class in global namespace because there can be some -// problems when we have multiple versions of Mutex in each shared object. -// -// NOTE: by default, we have #ifdef'ed out the TryLock() method. -// This is for two reasons: -// 1) TryLock() under Windows is a bit annoying (it requires a -// #define to be defined very early). -// 2) TryLock() is broken for NO_THREADS mode, at least in NDEBUG -// mode. -// If you need TryLock(), and either these two caveats are not a -// problem for you, or you're willing to work around them, then -// feel free to #define GMUTEX_TRYLOCK, or to remove the #ifdefs -// in the code below. -// -// CYGWIN NOTE: Cygwin support for rwlock seems to be buggy: -// http://www.cygwin.com/ml/cygwin/2008-12/msg00017.html -// Because of that, we might as well use windows locks for -// cygwin. They seem to be more reliable than the cygwin pthreads layer. -// -// TRICKY IMPLEMENTATION NOTE: -// This class is designed to be safe to use during -// dynamic-initialization -- that is, by global constructors that are -// run before main() starts. The issue in this case is that -// dynamic-initialization happens in an unpredictable order, and it -// could be that someone else's dynamic initializer could call a -// function that tries to acquire this mutex -- but that all happens -// before this mutex's constructor has run. (This can happen even if -// the mutex and the function that uses the mutex are in the same .cc -// file.) Basically, because Mutex does non-trivial work in its -// constructor, it's not, in the naive implementation, safe to use -// before dynamic initialization has run on it. -// -// The solution used here is to pair the actual mutex primitive with a -// bool that is set to true when the mutex is dynamically initialized. -// (Before that it's false.) Then we modify all mutex routines to -// look at the bool, and not try to lock/unlock until the bool makes -// it to true (which happens after the Mutex constructor has run.) -// -// This works because before main() starts -- particularly, during -// dynamic initialization -- there are no threads, so a) it's ok that -// the mutex operations are a no-op, since we don't need locking then -// anyway; and b) we can be quite confident our bool won't change -// state between a call to Lock() and a call to Unlock() (that would -// require a global constructor in one translation unit to call Lock() -// and another global constructor in another translation unit to call -// Unlock() later, which is pretty perverse). -// -// That said, it's tricky, and can conceivably fail; it's safest to -// avoid trying to acquire a mutex in a global constructor, if you -// can. One way it can fail is that a really smart compiler might -// initialize the bool to true at static-initialization time (too -// early) rather than at dynamic-initialization time. To discourage -// that, we set is_safe_ to true in code (not the constructor -// colon-initializer) and set it to true via a function that always -// evaluates to true, but that the compiler can't know always -// evaluates to true. This should be good enough. - -#ifndef GOOGLE_MUTEX_H_ -#define GOOGLE_MUTEX_H_ - -#include "config.h" // to figure out pthreads support - -#if defined(NO_THREADS) -typedef int MutexType; // to keep a lock-count -#elif defined(_WIN32) || defined(__CYGWIN__) -# ifndef WIN32_LEAN_AND_MEAN -# define WIN32_LEAN_AND_MEAN // We only need minimal includes -# endif -# ifdef GMUTEX_TRYLOCK -// We need Windows NT or later for TryEnterCriticalSection(). If you -// don't need that functionality, you can remove these _WIN32_WINNT -// lines, and change TryLock() to assert(0) or something. -# ifndef _WIN32_WINNT -# define _WIN32_WINNT 0x0400 -# endif -# endif -// To avoid macro definition of ERROR. -# ifndef NOGDI -# define NOGDI -# endif -// To avoid macro definition of min/max. -# ifndef NOMINMAX -# define NOMINMAX -# endif -# include -typedef CRITICAL_SECTION MutexType; -#elif defined(HAVE_PTHREAD) && defined(HAVE_RWLOCK) -// Needed for pthread_rwlock_*. If it causes problems, you could take it -// out, but then you'd have to unset HAVE_RWLOCK (at least on linux -- it -// *does* cause problems for FreeBSD, or MacOSX, but isn't needed -// for locking there.) -# ifdef __linux__ -# ifndef _XOPEN_SOURCE // Some other header might have already set it for - // us. -# define _XOPEN_SOURCE 500 // may be needed to get the rwlock calls -# endif -# endif -# include -using MutexType = pthread_rwlock_t; -#elif defined(HAVE_PTHREAD) -# include -typedef pthread_mutex_t MutexType; -#else -# error Need to implement mutex.h for your architecture, or #define NO_THREADS -#endif - -// We need to include these header files after defining _XOPEN_SOURCE -// as they may define the _XOPEN_SOURCE macro. -#include -#include // for abort() - -#define MUTEX_NAMESPACE glog_internal_namespace_ - -namespace MUTEX_NAMESPACE { - -class Mutex { - public: - // Create a Mutex that is not held by anybody. This constructor is - // typically used for Mutexes allocated on the heap or the stack. - // See below for a recommendation for constructing global Mutex - // objects. - inline Mutex(); - - // Destructor - inline ~Mutex(); - - inline void Lock(); // Block if needed until free then acquire exclusively - inline void Unlock(); // Release a lock acquired via Lock() -#ifdef GMUTEX_TRYLOCK - inline bool TryLock(); // If free, Lock() and return true, else return false -#endif - // Note that on systems that don't support read-write locks, these may - // be implemented as synonyms to Lock() and Unlock(). So you can use - // these for efficiency, but don't use them anyplace where being able - // to do shared reads is necessary to avoid deadlock. - inline void ReaderLock(); // Block until free or shared then acquire a share - inline void ReaderUnlock(); // Release a read share of this Mutex - inline void WriterLock() { Lock(); } // Acquire an exclusive lock - inline void WriterUnlock() { Unlock(); } // Release a lock from WriterLock() - - // TODO(hamaji): Do nothing, implement correctly. - inline void AssertHeld() {} - - private: - MutexType mutex_; - // We want to make sure that the compiler sets is_safe_ to true only - // when we tell it to, and never makes assumptions is_safe_ is - // always true. volatile is the most reliable way to do that. - volatile bool is_safe_; - - inline void SetIsSafe() { is_safe_ = true; } - - // Catch the error of writing Mutex when intending MutexLock. - explicit Mutex(Mutex* /*ignored*/) {} - // Disallow "evil" constructors - Mutex(const Mutex&) = delete; - void operator=(const Mutex&) = delete; -}; - -// Now the implementation of Mutex for various systems -#if defined(NO_THREADS) - -// When we don't have threads, we can be either reading or writing, -// but not both. We can have lots of readers at once (in no-threads -// mode, that's most likely to happen in recursive function calls), -// but only one writer. We represent this by having mutex_ be -1 when -// writing and a number > 0 when reading (and 0 when no lock is held). -// -// In debug mode, we assert these invariants, while in non-debug mode -// we do nothing, for efficiency. That's why everything is in an -// assert. - -Mutex::Mutex() : mutex_(0) {} -Mutex::~Mutex() { assert(mutex_ == 0); } -void Mutex::Lock() { assert(--mutex_ == -1); } -void Mutex::Unlock() { assert(mutex_++ == -1); } -# ifdef GMUTEX_TRYLOCK -bool Mutex::TryLock() { - if (mutex_) return false; - Lock(); - return true; -} -# endif -void Mutex::ReaderLock() { assert(++mutex_ > 0); } -void Mutex::ReaderUnlock() { assert(mutex_-- > 0); } - -#elif defined(_WIN32) || defined(__CYGWIN__) - -Mutex::Mutex() { - InitializeCriticalSection(&mutex_); - SetIsSafe(); -} -Mutex::~Mutex() { DeleteCriticalSection(&mutex_); } -void Mutex::Lock() { - if (is_safe_) EnterCriticalSection(&mutex_); -} -void Mutex::Unlock() { - if (is_safe_) LeaveCriticalSection(&mutex_); -} -# ifdef GMUTEX_TRYLOCK -bool Mutex::TryLock() { - return is_safe_ ? TryEnterCriticalSection(&mutex_) != 0 : true; -} -# endif -void Mutex::ReaderLock() { Lock(); } // we don't have read-write locks -void Mutex::ReaderUnlock() { Unlock(); } - -#elif defined(HAVE_PTHREAD) && defined(HAVE_RWLOCK) - -# define SAFE_PTHREAD(fncall) \ - do { /* run fncall if is_safe_ is true */ \ - if (is_safe_ && fncall(&mutex_) != 0) abort(); \ - } while (0) - -Mutex::Mutex() { - SetIsSafe(); - if (is_safe_ && pthread_rwlock_init(&mutex_, nullptr) != 0) abort(); -} -Mutex::~Mutex() { SAFE_PTHREAD(pthread_rwlock_destroy); } -void Mutex::Lock() { SAFE_PTHREAD(pthread_rwlock_wrlock); } -void Mutex::Unlock() { SAFE_PTHREAD(pthread_rwlock_unlock); } -# ifdef GMUTEX_TRYLOCK -bool Mutex::TryLock() { - return is_safe_ ? pthread_rwlock_trywrlock(&mutex_) == 0 : true; -} -# endif -void Mutex::ReaderLock() { SAFE_PTHREAD(pthread_rwlock_rdlock); } -void Mutex::ReaderUnlock() { SAFE_PTHREAD(pthread_rwlock_unlock); } -# undef SAFE_PTHREAD - -#elif defined(HAVE_PTHREAD) - -# define SAFE_PTHREAD(fncall) \ - do { /* run fncall if is_safe_ is true */ \ - if (is_safe_ && fncall(&mutex_) != 0) abort(); \ - } while (0) - -Mutex::Mutex() { - SetIsSafe(); - if (is_safe_ && pthread_mutex_init(&mutex_, nullptr) != 0) abort(); -} -Mutex::~Mutex() { SAFE_PTHREAD(pthread_mutex_destroy); } -void Mutex::Lock() { SAFE_PTHREAD(pthread_mutex_lock); } -void Mutex::Unlock() { SAFE_PTHREAD(pthread_mutex_unlock); } -# ifdef GMUTEX_TRYLOCK -bool Mutex::TryLock() { - return is_safe_ ? pthread_mutex_trylock(&mutex_) == 0 : true; -} -# endif -void Mutex::ReaderLock() { Lock(); } -void Mutex::ReaderUnlock() { Unlock(); } -# undef SAFE_PTHREAD - -#endif - -// -------------------------------------------------------------------------- -// Some helper classes - -// MutexLock(mu) acquires mu when constructed and releases it when destroyed. -class MutexLock { - public: - explicit MutexLock(Mutex* mu) : mu_(mu) { mu_->Lock(); } - ~MutexLock() { mu_->Unlock(); } - - private: - Mutex* const mu_; - // Disallow "evil" constructors - MutexLock(const MutexLock&) = delete; - void operator=(const MutexLock&) = delete; -}; - -// ReaderMutexLock and WriterMutexLock do the same, for rwlocks -class ReaderMutexLock { - public: - explicit ReaderMutexLock(Mutex* mu) : mu_(mu) { mu_->ReaderLock(); } - ~ReaderMutexLock() { mu_->ReaderUnlock(); } - - private: - Mutex* const mu_; - // Disallow "evil" constructors - ReaderMutexLock(const ReaderMutexLock&) = delete; - void operator=(const ReaderMutexLock&) = delete; -}; - -class WriterMutexLock { - public: - explicit WriterMutexLock(Mutex* mu) : mu_(mu) { mu_->WriterLock(); } - ~WriterMutexLock() { mu_->WriterUnlock(); } - - private: - Mutex* const mu_; - // Disallow "evil" constructors - WriterMutexLock(const WriterMutexLock&) = delete; - void operator=(const WriterMutexLock&) = delete; -}; - -// Catch bug where variable name is omitted, e.g. MutexLock (&mu); -#define MutexLock(x) COMPILE_ASSERT(0, mutex_lock_decl_missing_var_name) -#define ReaderMutexLock(x) COMPILE_ASSERT(0, rmutex_lock_decl_missing_var_name) -#define WriterMutexLock(x) COMPILE_ASSERT(0, wmutex_lock_decl_missing_var_name) - -} // namespace MUTEX_NAMESPACE - -using namespace MUTEX_NAMESPACE; - -#undef MUTEX_NAMESPACE - -#endif /* #define GOOGLE_MUTEX_H__ */ diff --git a/src/config.h.cmake.in b/src/config.h.cmake.in index 1dc8baabd..7716d1fe4 100644 --- a/src/config.h.cmake.in +++ b/src/config.h.cmake.in @@ -46,9 +46,6 @@ /* Define if you have the 'pwrite' function */ #cmakedefine HAVE_PWRITE -/* define if the compiler implements pthread_rwlock_* */ -#cmakedefine HAVE_RWLOCK - /* Define if you have the 'sigaction' function */ #cmakedefine HAVE_SIGACTION diff --git a/src/logging.cc b/src/logging.cc index 4607402b2..418f71c5c 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -36,6 +36,8 @@ #include #include #include +#include +#include #include #include "base/commandlineflags.h" // to get the program name @@ -404,7 +406,7 @@ struct LogMessage::LogMessageData { // changing the destination file for log messages of a given severity) also // lock this mutex. Please be sure that anybody who might possibly need to // lock it does so. -static Mutex log_mutex; +static std::mutex log_mutex; // Number of messages sent at each severity. Under log_mutex. int64 LogMessage::num_messages_[NUM_SEVERITIES] = {0, 0, 0, 0}; @@ -457,7 +459,7 @@ class LogFileObject : public base::Logger { // It is the actual file length for the system loggers, // i.e., INFO, ERROR, etc. uint32 LogSize() override { - MutexLock l(&lock_); + std::lock_guard l{mutex_}; return file_length_; } @@ -469,7 +471,7 @@ class LogFileObject : public base::Logger { private: static const uint32 kRolloverAttemptFrequency = 0x20; - Mutex lock_; + std::mutex mutex_; bool base_filename_selected_; string base_filename_; string symlink_basename_; @@ -560,6 +562,17 @@ class LogDestination { static void DeleteLogDestinations(); private: +#if defined(__cpp_lib_shared_mutex) && (__cpp_lib_shared_mutex >= 201505L) + // Use untimed shared mutex + using SinkMutex = std::shared_mutex; + using SinkLock = std::lock_guard; +#else // !(defined(__cpp_lib_shared_mutex) && (__cpp_lib_shared_mutex >= + // 201505L)) Fallback to timed shared mutex + using SinkMutex = std::shared_timed_mutex; + using SinkLock = std::unique_lock; +#endif // defined(__cpp_lib_shared_mutex) && (__cpp_lib_shared_mutex >= + // 201505L) + LogDestination(LogSeverity severity, const char* base_filename); ~LogDestination(); @@ -612,7 +625,7 @@ class LogDestination { // Protects the vector sinks_, // but not the LogSink objects its elements reference. - static Mutex sink_mutex_; + static SinkMutex sink_mutex_; // Disallow LogDestination(const LogDestination&) = delete; @@ -626,7 +639,7 @@ string LogDestination::addresses_; string LogDestination::hostname_; vector* LogDestination::sinks_ = nullptr; -Mutex LogDestination::sink_mutex_; +LogDestination::SinkMutex LogDestination::sink_mutex_; bool LogDestination::terminal_supports_color_ = TerminalSupportsColor(); /* static */ @@ -674,7 +687,7 @@ inline void LogDestination::FlushLogFilesUnsafe(int min_severity) { inline void LogDestination::FlushLogFiles(int min_severity) { // Prevent any subtle race conditions by wrapping a mutex lock around // all this stuff. - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; for (int i = min_severity; i < NUM_SEVERITIES; i++) { LogDestination* log = log_destination(i); if (log != nullptr) { @@ -688,7 +701,7 @@ inline void LogDestination::SetLogDestination(LogSeverity severity, assert(severity >= 0 && severity < NUM_SEVERITIES); // Prevent any subtle race conditions by wrapping a mutex lock around // all this stuff. - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; log_destination(severity)->fileobject_.SetBasename(base_filename); } @@ -696,14 +709,14 @@ inline void LogDestination::SetLogSymlink(LogSeverity severity, const char* symlink_basename) { CHECK_GE(severity, 0); CHECK_LT(severity, NUM_SEVERITIES); - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; log_destination(severity)->fileobject_.SetSymlinkBasename(symlink_basename); } inline void LogDestination::AddLogSink(LogSink* destination) { // Prevent any subtle race conditions by wrapping a mutex lock around // all this stuff. - MutexLock l(&sink_mutex_); + SinkLock l{sink_mutex_}; if (!sinks_) sinks_ = new vector; sinks_->push_back(destination); } @@ -711,7 +724,7 @@ inline void LogDestination::AddLogSink(LogSink* destination) { inline void LogDestination::RemoveLogSink(LogSink* destination) { // Prevent any subtle race conditions by wrapping a mutex lock around // all this stuff. - MutexLock l(&sink_mutex_); + SinkLock l{sink_mutex_}; // This doesn't keep the sinks in order, but who cares? if (sinks_) { sinks_->erase(std::remove(sinks_->begin(), sinks_->end(), destination), @@ -722,7 +735,7 @@ inline void LogDestination::RemoveLogSink(LogSink* destination) { inline void LogDestination::SetLogFilenameExtension(const char* ext) { // Prevent any subtle race conditions by wrapping a mutex lock around // all this stuff. - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; for (int severity = 0; severity < NUM_SEVERITIES; ++severity) { log_destination(severity)->fileobject_.SetExtension(ext); } @@ -732,7 +745,7 @@ inline void LogDestination::SetStderrLogging(LogSeverity min_severity) { assert(min_severity >= 0 && min_severity < NUM_SEVERITIES); // Prevent any subtle race conditions by wrapping a mutex lock around // all this stuff. - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; FLAGS_stderrthreshold = min_severity; } @@ -750,7 +763,7 @@ inline void LogDestination::SetEmailLogging(LogSeverity min_severity, assert(min_severity >= 0 && min_severity < NUM_SEVERITIES); // Prevent any subtle race conditions by wrapping a mutex lock around // all this stuff. - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; LogDestination::email_logging_severity_ = min_severity; LogDestination::addresses_ = addresses; } @@ -898,7 +911,7 @@ inline void LogDestination::LogToSinks(LogSeverity severity, const LogMessageTime& logmsgtime, const char* message, size_t message_len) { - ReaderMutexLock l(&sink_mutex_); + std::shared_lock l{sink_mutex_}; if (sinks_) { for (size_t i = sinks_->size(); i-- > 0;) { (*sinks_)[i]->send(severity, full_filename, base_filename, line, @@ -908,7 +921,7 @@ inline void LogDestination::LogToSinks(LogSeverity severity, } inline void LogDestination::WaitForSinks(LogMessage::LogMessageData* data) { - ReaderMutexLock l(&sink_mutex_); + std::shared_lock l{sink_mutex_}; if (sinks_) { for (size_t i = sinks_->size(); i-- > 0;) { (*sinks_)[i]->WaitTillSent(); @@ -937,7 +950,7 @@ void LogDestination::DeleteLogDestinations() { delete log_destination; log_destination = nullptr; } - MutexLock l(&sink_mutex_); + SinkLock l{sink_mutex_}; delete sinks_; sinks_ = nullptr; } @@ -988,7 +1001,7 @@ LogFileObject::LogFileObject(LogSeverity severity, const char* base_filename) } LogFileObject::~LogFileObject() { - MutexLock l(&lock_); + std::lock_guard l{mutex_}; if (file_ != nullptr) { fclose(file_); file_ = nullptr; @@ -996,7 +1009,7 @@ LogFileObject::~LogFileObject() { } void LogFileObject::SetBasename(const char* basename) { - MutexLock l(&lock_); + std::lock_guard l{mutex_}; base_filename_selected_ = true; if (base_filename_ != basename) { // Get rid of old log file since we are changing names @@ -1010,7 +1023,7 @@ void LogFileObject::SetBasename(const char* basename) { } void LogFileObject::SetExtension(const char* ext) { - MutexLock l(&lock_); + std::lock_guard l{mutex_}; if (filename_extension_ != ext) { // Get rid of old log file since we are changing names if (file_ != nullptr) { @@ -1023,12 +1036,12 @@ void LogFileObject::SetExtension(const char* ext) { } void LogFileObject::SetSymlinkBasename(const char* symlink_basename) { - MutexLock l(&lock_); + std::lock_guard l{mutex_}; symlink_basename_ = symlink_basename; } void LogFileObject::Flush() { - MutexLock l(&lock_); + std::lock_guard l{mutex_}; FlushUnlocked(); } @@ -1150,7 +1163,7 @@ bool LogFileObject::CreateLogfile(const string& time_pid_string) { void LogFileObject::Write(bool force_flush, time_t timestamp, const char* message, size_t message_len) { - MutexLock l(&lock_); + std::lock_guard l{mutex_}; // We don't log if the base_name_ is "" (which means "don't write") if (base_filename_selected_ && base_filename_.empty()) { @@ -1536,7 +1549,7 @@ bool LogCleaner::IsLogLastModifiedOver(const string& filepath, // the data from the first call, we allocate two sets of space. One // for exclusive use by the first thread, and one for shared use by // all other threads. -static Mutex fatal_msg_lock; +static std::mutex fatal_msg_lock; static CrashReason crash_reason; static bool fatal_msg_exclusive = true; static LogMessage::LogMessageData fatal_msg_data_exclusive; @@ -1626,7 +1639,7 @@ void LogMessage::Init(const char* file, int line, LogSeverity severity, #endif // defined(GLOG_THREAD_LOCAL_STORAGE) data_->first_fatal_ = false; } else { - MutexLock l(&fatal_msg_lock); + std::lock_guard l{fatal_msg_lock}; if (fatal_msg_exclusive) { fatal_msg_exclusive = false; data_ = &fatal_msg_data_exclusive; @@ -1750,7 +1763,7 @@ void LogMessage::Flush() { // Prevent any subtle race conditions by wrapping a mutex lock around // the actual logging action per se. { - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; (this->*(data_->send_method_))(); ++num_messages_[static_cast(data_->severity_)]; } @@ -1797,7 +1810,6 @@ void ReprintFatalMessage() { void LogMessage::SendToLog() EXCLUSIVE_LOCKS_REQUIRED(log_mutex) { static bool already_warned_before_initgoogle = false; - log_mutex.AssertHeld(); RAW_DCHECK(data_->num_chars_to_log_ > 0 && data_->message_text_[data_->num_chars_to_log_ - 1] == '\n', @@ -1879,7 +1891,7 @@ void LogMessage::SendToLog() EXCLUSIVE_LOCKS_REQUIRED(log_mutex) { // can use the logging facility. Alternately, we could add // an entire unsafe logging interface to bypass locking // for signal handlers but this seems simpler. - log_mutex.Unlock(); + log_mutex.unlock(); LogDestination::WaitForSinks(data_); const char* message = "*** Check failure stack trace: ***\n"; @@ -2001,18 +2013,18 @@ void LogMessage::SendToSyslogAndLog() { } base::Logger* base::GetLogger(LogSeverity severity) { - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; return LogDestination::log_destination(severity)->GetLoggerImpl(); } void base::SetLogger(LogSeverity severity, base::Logger* logger) { - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; LogDestination::log_destination(severity)->SetLoggerImpl(logger); } // L < log_mutex. Acquires and releases mutex_. int64 LogMessage::num_messages(int severity) { - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; return num_messages_[severity]; } @@ -2145,7 +2157,7 @@ namespace internal { bool GetExitOnDFatal(); bool GetExitOnDFatal() { - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; return exit_on_dfatal; } @@ -2161,7 +2173,7 @@ bool GetExitOnDFatal() { // these differences are acceptable. void SetExitOnDFatal(bool value); void SetExitOnDFatal(bool value) { - MutexLock l(&log_mutex); + std::lock_guard l{log_mutex}; exit_on_dfatal = value; } diff --git a/src/logging_unittest.cc b/src/logging_unittest.cc index 6c254f1fe..cb1c657e7 100644 --- a/src/logging_unittest.cc +++ b/src/logging_unittest.cc @@ -50,6 +50,7 @@ #include #include #include +#include #include #include #include @@ -1196,29 +1197,29 @@ class TestLogSinkWriter : public Thread { // Just buffer it (can't use LOG() here). void Buffer(const string& message) { - mutex_.Lock(); + mutex_.lock(); RAW_LOG(INFO, "Buffering"); messages_.push(message); - mutex_.Unlock(); + mutex_.unlock(); RAW_LOG(INFO, "Buffered"); } // Wait for the buffer to clear (can't use LOG() here). void Wait() { RAW_LOG(INFO, "Waiting"); - mutex_.Lock(); + mutex_.lock(); while (!NoWork()) { - mutex_.Unlock(); + mutex_.unlock(); SleepForMilliseconds(1); - mutex_.Lock(); + mutex_.lock(); } RAW_LOG(INFO, "Waited"); - mutex_.Unlock(); + mutex_.unlock(); } // Trigger thread exit. void Stop() { - MutexLock l(&mutex_); + std::lock_guard l(mutex_); should_exit_ = true; } @@ -1232,14 +1233,14 @@ class TestLogSinkWriter : public Thread { // Thread body; CAN use LOG() here! void Run() override { while (true) { - mutex_.Lock(); + mutex_.lock(); while (!HaveWork()) { - mutex_.Unlock(); + mutex_.unlock(); SleepForMilliseconds(1); - mutex_.Lock(); + mutex_.lock(); } if (should_exit_ && messages_.empty()) { - mutex_.Unlock(); + mutex_.unlock(); break; } // Give the main thread time to log its message, @@ -1253,7 +1254,7 @@ class TestLogSinkWriter : public Thread { // where LOG() usage can't be eliminated, // e.g. pushing the message over with an RPC: size_t messages_left = messages_.size(); - mutex_.Unlock(); + mutex_.unlock(); SleepForMilliseconds(20); // May not use LOG while holding mutex_, because Buffer() // acquires mutex_, and Buffer is called from LOG(), @@ -1267,7 +1268,7 @@ class TestLogSinkWriter : public Thread { // data --------------- - Mutex mutex_; + std::mutex mutex_; bool should_exit_{false}; queue messages_; // messages to be logged }; diff --git a/src/utilities.h b/src/utilities.h index f19c63046..50605cf95 100644 --- a/src/utilities.h +++ b/src/utilities.h @@ -54,7 +54,6 @@ #include -#include "base/mutex.h" // This must go first so we get _XOPEN_SOURCE #include "glog/logging.h" #if defined(GLOG_OS_WINDOWS) diff --git a/src/vlog_is_on.cc b/src/vlog_is_on.cc index b514acf51..4c4974a05 100644 --- a/src/vlog_is_on.cc +++ b/src/vlog_is_on.cc @@ -36,6 +36,7 @@ #include #include #include +#include #include #include "base/commandlineflags.h" @@ -121,7 +122,7 @@ struct VModuleInfo { }; // This protects the following global variables. -static Mutex vmodule_lock; +static std::mutex vmodule_mutex; // Pointer to head of the VModuleInfo list. // It's a map from module pattern to logging level for those module(s). static VModuleInfo* vmodule_list = nullptr; @@ -130,9 +131,8 @@ static SiteFlag* cached_site_list = nullptr; // Boolean initialization flag. static bool inited_vmodule = false; -// L >= vmodule_lock. +// L >= vmodule_mutex. static void VLOG2Initializer() { - vmodule_lock.AssertHeld(); // Can now parse --vmodule flag and initialize mapping of module-specific // logging levels. inited_vmodule = false; @@ -172,7 +172,8 @@ int SetVLOGLevel(const char* module_pattern, int log_level) { size_t const pattern_len = strlen(module_pattern); bool found = false; { - MutexLock l(&vmodule_lock); // protect whole read-modify-write + std::lock_guard l( + vmodule_mutex); // protect whole read-modify-write for (const VModuleInfo* info = vmodule_list; info != nullptr; info = info->next) { if (info->module_pattern == module_pattern) { @@ -221,7 +222,7 @@ int SetVLOGLevel(const char* module_pattern, int log_level) { // NOTE: This function must not allocate memory or require any locks. bool InitVLOG3__(SiteFlag* site_flag, int32* level_default, const char* fname, int32 verbose_level) { - MutexLock l(&vmodule_lock); + std::lock_guard l(vmodule_mutex); bool read_vmodule_flag = inited_vmodule; if (!read_vmodule_flag) { VLOG2Initializer();