From c0c5f0ed0ae022e5ea927c06391c373a71805c8d Mon Sep 17 00:00:00 2001 From: themuffinator Date: Sun, 23 Nov 2025 21:25:05 +0000 Subject: [PATCH] Improve logger thread safety --- src/shared/logger.cpp | 49 ++++++++++--- tests/test_logger_thread_safety.cpp | 103 ++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 10 deletions(-) create mode 100644 tests/test_logger_thread_safety.cpp diff --git a/src/shared/logger.cpp b/src/shared/logger.cpp index e85f71a6..7f3b7c7c 100644 --- a/src/shared/logger.cpp +++ b/src/shared/logger.cpp @@ -1,10 +1,12 @@ #include "logger.hpp" +#include #include #include #include #include #include +#include #include #include @@ -12,10 +14,17 @@ namespace worr { namespace { std::string g_module_name; -LogLevel g_log_level = LogLevel::Info; +std::atomic g_log_level = LogLevel::Info; +std::mutex g_logger_mutex; void (*g_print_sink)(const char*) = nullptr; void (*g_error_sink)(const char*) = nullptr; +struct LoggerConfig { + std::string module_name; + void (*print_sink)(const char*); + void (*error_sink)(const char*); +}; + /* ============= ParseLogLevel @@ -100,19 +109,33 @@ FormatMessage Build a structured log message for output. ============= */ -std::string FormatMessage(LogLevel level, std::string_view message) +std::string FormatMessage(LogLevel level, std::string_view module_name, std::string_view message) { static constexpr std::array prefixes{ "[TRACE]", "[DEBUG]", "[INFO]", "[WARN]", "[ERROR]" }; const size_t prefix_index = static_cast(LevelWeight(level)); std::string_view level_label = prefixes[std::min(prefix_index, prefixes.size() - 1)]; - std::string formatted = std::format("[WORR][{}] {} {}", g_module_name, level_label, message); + std::string formatted = std::format("[WORR][{}] {} {}", module_name, level_label, message); if (!formatted.empty() && formatted.back() != '\n') formatted.push_back('\n'); return formatted; } +/* +============= +GetLoggerConfig + +Capture current logger configuration under mutex protection. +============= +*/ +LoggerConfig GetLoggerConfig() +{ + std::scoped_lock lock(g_logger_mutex); + + return { g_module_name, g_print_sink, g_error_sink }; +} + } // namespace /* @@ -124,10 +147,12 @@ Initialize the logger with module metadata and output sinks. */ void InitLogger(std::string_view module_name, void (*print_sink)(const char*), void (*error_sink)(const char*)) { + std::scoped_lock lock(g_logger_mutex); + g_module_name = module_name; g_print_sink = print_sink; g_error_sink = error_sink; - g_log_level = ReadLogLevelFromEnv(); + g_log_level.store(ReadLogLevelFromEnv(), std::memory_order_relaxed); } /* @@ -139,7 +164,7 @@ Override the current logging level programmatically. */ void SetLogLevel(LogLevel level) { - g_log_level = level; + g_log_level.store(level, std::memory_order_relaxed); } /* @@ -151,7 +176,7 @@ Fetch the currently active log level. */ LogLevel GetLogLevel() { - return g_log_level; + return g_log_level.load(std::memory_order_relaxed); } /* @@ -163,7 +188,8 @@ Return whether the provided log level should emit output. */ bool IsLogLevelEnabled(LogLevel level) { - return LevelWeight(level) >= LevelWeight(g_log_level); + const LogLevel current_level = g_log_level.load(std::memory_order_relaxed); + return LevelWeight(level) >= LevelWeight(current_level); } /* @@ -188,8 +214,10 @@ Hook-compatible error printer that always emits output. */ void LoggerError(const char* message) { + const LoggerConfig config = GetLoggerConfig(); + Log(LogLevel::Error, message); - EnsureSink(g_error_sink, FormatMessage(LogLevel::Error, message)); + EnsureSink(config.error_sink, FormatMessage(LogLevel::Error, config.module_name, message)); } /* @@ -202,9 +230,10 @@ Log a pre-formatted message if the level is enabled. void Log(LogLevel level, std::string_view message) { if (!IsLogLevelEnabled(level)) - return; + return; - EnsureSink(g_print_sink, FormatMessage(level, message)); + const LoggerConfig config = GetLoggerConfig(); + EnsureSink(config.print_sink, FormatMessage(level, config.module_name, message)); } /* diff --git a/tests/test_logger_thread_safety.cpp b/tests/test_logger_thread_safety.cpp new file mode 100644 index 00000000..53c7c1ec --- /dev/null +++ b/tests/test_logger_thread_safety.cpp @@ -0,0 +1,103 @@ +#include "shared/logger.hpp" + +#include +#include +#include +#include +#include +#include + +namespace { + std::mutex g_sinkMutex; + std::vector g_printMessages; + std::vector g_errorMessages; + +/* +============= +CollectPrint + +Store a print sink message for verification. +============= +*/ + void CollectPrint(const char* message) +{ + std::scoped_lock lock(g_sinkMutex); + g_printMessages.emplace_back(message); + } + +/* +============= +CollectError + +Store an error sink message for verification. +============= +*/ + void CollectError(const char* message) +{ + std::scoped_lock lock(g_sinkMutex); + g_errorMessages.emplace_back(message); + } + +/* +============= +ToggleLevels + +Repeatedly adjust the log level to exercise atomic coordination. +============= +*/ + void ToggleLevels() +{ + for (int i = 0; i < 200; ++i) { + worr::SetLogLevel((i % 2) == 0 ? worr::LogLevel::Info : worr::LogLevel::Trace); + } + } + +/* +============= +LogMessages + +Emit informational and error messages while configuration changes. +============= +*/ + void LogMessages() +{ + for (int i = 0; i < 200; ++i) { + worr::Log(worr::LogLevel::Info, "concurrent-info"); + worr::Log(worr::LogLevel::Error, "concurrent-error"); + } + } +} // namespace + +/* +============= +main + +Verify concurrent logging preserves configuration integrity. +============= +*/ +int main() +{ + worr::InitLogger("threaded", &CollectPrint, &CollectError); + + std::thread levelThread(&ToggleLevels); + std::thread logThreadA(&LogMessages); + std::thread logThreadB(&LogMessages); + + levelThread.join(); + logThreadA.join(); + logThreadB.join(); + + std::scoped_lock lock(g_sinkMutex); + assert(!g_printMessages.empty()); + assert(!g_errorMessages.empty()); + + const std::string prefix = "[WORR][threaded]"; + for (const std::string& message : g_printMessages) { + assert(message.rfind(prefix, 0) == 0); + } + for (const std::string& message : g_errorMessages) { + assert(message.rfind(prefix, 0) == 0); + } + + return 0; +}