Merge "Remove thread safety from libbase logging / liblog" into rvc-dev
diff --git a/base/logging.cpp b/base/logging.cpp
index cd460eb..3c73fea 100644
--- a/base/logging.cpp
+++ b/base/logging.cpp
@@ -195,7 +195,6 @@
return logging_lock;
}
-// Only used for Q fallback.
static LogFunction& Logger() {
#ifdef __ANDROID__
static auto& logger = *new LogFunction(LogdLogger());
@@ -205,7 +204,6 @@
return logger;
}
-// Only used for Q fallback.
static AbortFunction& Aborter() {
static auto& aborter = *new AbortFunction(DefaultAborter);
return aborter;
@@ -416,45 +414,27 @@
}
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); });
}
}
diff --git a/liblog/logger_write.cpp b/liblog/logger_write.cpp
index 74b0ab9..b1bed80 100644
--- a/liblog/logger_write.cpp
+++ b/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 @@
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 @@
#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 @@
}
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 @@
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 @@
}
#endif
- auto lock = std::shared_lock{logger_function_lock};
logger_function(log_message);
}
diff --git a/liblog/logger_write.h b/liblog/logger_write.h
index 065fd55..eee2778 100644
--- a/liblog/logger_write.h
+++ b/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;
\ No newline at end of file
+std::string& GetDefaultTag();
diff --git a/liblog/properties.cpp b/liblog/properties.cpp
index abd48fc..37670ec 100644
--- a/liblog/properties.cpp
+++ b/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 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();