Merge "Remove thread safety from libbase logging / liblog" (538bac2e) · Commits · e / os / android_system_core

base/logging.cpp

+7 −27

Original line number	Diff line number	Diff line
		@@ -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

+0 −12

Original line number	Diff line number	Diff line
		@@ -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

+1 −4

Original line number	Diff line number	Diff line
		@@ -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();

liblog/properties.cpp

+0 −3

Original line number	Diff line number	Diff line
		@@ -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();