Merge "Remove thread safety from libbase logging / liblog" into rvc-dev am: c53c64c0ec am: 01cd4c654a

Change-Id: I3e217bf888f8f6003f5447d941a20f1cba811573

base/logging.cpp

@@ -195,7 +195,6 @@ static std::mutex& LoggingLock() {
   return logging_lock;
 }
 
-// Only used for Q fallback.
 static LogFunction& Logger() {
 #ifdef __ANDROID__
   static auto& logger = *new LogFunction(LogdLogger());
@@ -205,7 +204,6 @@ static LogFunction& Logger() {
   return logger;
 }
 
-// Only used for Q fallback.
 static AbortFunction& Aborter() {
   static auto& aborter = *new AbortFunction(DefaultAborter);
   return aborter;
@@ -416,45 +414,27 @@ void InitLogging(char* argv[], LogFunction&& logger, AbortFunction&& aborter) {
 }
 
 void SetLogger(LogFunction&& logger) {
+  Logger() = std::move(logger);
+
   static auto& liblog_functions = GetLibLogFunctions();
   if (liblog_functions) {
-    // We need to atomically swap the old and new pointers since other threads may be logging.
-    // We know all threads will be using the new logger after __android_log_set_logger() returns,
-    // so we can delete it then.
-    // This leaks one std::function<> per instance of libbase if multiple copies of libbase within a
-    // single process call SetLogger().  That is the same cost as having a static
-    // std::function<>, which is the not-thread-safe alternative.
-    static std::atomic<LogFunction*> logger_function(nullptr);
-    auto* old_logger_function = logger_function.exchange(new LogFunction(logger));
     liblog_functions->__android_log_set_logger([](const struct __android_log_message* log_message) {
       auto log_id = log_id_tToLogId(log_message->buffer_id);
       auto severity = PriorityToLogSeverity(log_message->priority);
 
-      auto& function = *logger_function.load(std::memory_order_acquire);
-      function(log_id, severity, log_message->tag, log_message->file, log_message->line,
+      Logger()(log_id, severity, log_message->tag, log_message->file, log_message->line,
                log_message->message);
     });
-    delete old_logger_function;
-  } else {
-    std::lock_guard<std::mutex> lock(LoggingLock());
-    Logger() = std::move(logger);
   }
 }
 
 void SetAborter(AbortFunction&& aborter) {
+  Aborter() = std::move(aborter);
+
   static auto& liblog_functions = GetLibLogFunctions();
   if (liblog_functions) {
-    // See the comment in SetLogger().
-    static std::atomic<AbortFunction*> abort_function(nullptr);
-    auto* old_abort_function = abort_function.exchange(new AbortFunction(aborter));
-    liblog_functions->__android_log_set_aborter([](const char* abort_message) {
-      auto& function = *abort_function.load(std::memory_order_acquire);
-      function(abort_message);
-    });
-    delete old_abort_function;
-  } else {
-    std::lock_guard<std::mutex> lock(LoggingLock());
-    Aborter() = std::move(aborter);
+    liblog_functions->__android_log_set_aborter(
+        [](const char* abort_message) { Aborter()(abort_message); });
   }
 }
 

liblog/logger_write.cpp

@@ -28,7 +28,6 @@
 #endif
 
 #include <atomic>
-#include <shared_mutex>
 
 #include <android-base/errno_restorer.h>
 #include <android-base/macros.h>
@@ -38,7 +37,6 @@
 #include "android/log.h"
 #include "log/log_read.h"
 #include "logger.h"
-#include "rwlock.h"
 #include "uio.h"
 
 #ifdef __ANDROID__
@@ -142,10 +140,8 @@ std::string& GetDefaultTag() {
   static std::string default_tag = getprogname();
   return default_tag;
 }
-RwLock default_tag_lock;
 
 void __android_log_set_default_tag(const char* tag) {
-  auto lock = std::unique_lock{default_tag_lock};
   GetDefaultTag().assign(tag, 0, LOGGER_ENTRY_MAX_PAYLOAD);
 }
 
@@ -163,10 +159,8 @@ static __android_logger_function logger_function = __android_log_logd_logger;
 #else
 static __android_logger_function logger_function = __android_log_stderr_logger;
 #endif
-static RwLock logger_function_lock;
 
 void __android_log_set_logger(__android_logger_function logger) {
-  auto lock = std::unique_lock{logger_function_lock};
   logger_function = logger;
 }
 
@@ -180,15 +174,12 @@ void __android_log_default_aborter(const char* abort_message) {
 }
 
 static __android_aborter_function aborter_function = __android_log_default_aborter;
-static RwLock aborter_function_lock;
 
 void __android_log_set_aborter(__android_aborter_function aborter) {
-  auto lock = std::unique_lock{aborter_function_lock};
   aborter_function = aborter;
 }
 
 void __android_log_call_aborter(const char* abort_message) {
-  auto lock = std::shared_lock{aborter_function_lock};
   aborter_function(abort_message);
 }
 
@@ -310,9 +301,7 @@ void __android_log_write_log_message(__android_log_message* log_message) {
     return;
   }
 
-  auto tag_lock = std::shared_lock{default_tag_lock, std::defer_lock};
   if (log_message->tag == nullptr) {
-    tag_lock.lock();
     log_message->tag = GetDefaultTag().c_str();
   }
 
@@ -322,7 +311,6 @@ void __android_log_write_log_message(__android_log_message* log_message) {
   }
 #endif
 
-  auto lock = std::shared_lock{logger_function_lock};
   logger_function(log_message);
 }
 

liblog/logger_write.h

@@ -18,7 +18,4 @@
 
 #include <string>
 
-#include "rwlock.h"
-
-std::string& GetDefaultTag();  // Must read lock default_tag_lock
-extern RwLock default_tag_lock;
+std::string& GetDefaultTag();

liblog/properties.cpp

@@ -23,7 +23,6 @@
 #include <unistd.h>
 
 #include <algorithm>
-#include <shared_mutex>
 
 #include <private/android_logger.h>
 
@@ -99,9 +98,7 @@ static int __android_log_level(const char* tag, size_t len) {
   static const char log_namespace[] = "persist.log.tag.";
   static const size_t base_offset = 8; /* skip "persist." */
 
-  auto tag_lock = std::shared_lock{default_tag_lock, std::defer_lock};
   if (tag == nullptr || len == 0) {
-    tag_lock.lock();
     auto& tag_string = GetDefaultTag();
     tag = tag_string.c_str();
     len = tag_string.size();