From 116e4b23a794e8f0ecef75096ebb3d60427ac525 Mon Sep 17 00:00:00 2001
From: Jochen Topf <jochen@topf.org>
Date: Wed, 28 Jun 2023 19:46:44 +0200
Subject: [PATCH] Fix: Fix threading issue in logger

The (singleton) logger can be called from multiple threads at once, but
it writes the m_needs_leading_return member variable. This commit turns
that variable into an atomic to fix this.

This also moves some logic out of the generate_common_prefix() function
so that we can make it const. This makes it easier to see that the code
is now thread-safe.
---
 src/logging.cpp | 19 +++++--------------
 src/logging.hpp | 16 ++++++++++++----
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/src/logging.cpp b/src/logging.cpp
index 78c4df71a..7c890d103 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -19,28 +19,19 @@ logger the_logger{};
 /// Access the global logger singleton
 logger &get_logger() noexcept { return the_logger; }
 
-std::string logger::generate_common_prefix(fmt::text_style const &ts,
-                                           char const *prefix)
+void logger::generate_common_prefix(std::string *str, fmt::text_style const &ts,
+                                    char const *prefix) const
 {
-    std::string str;
-
-    if (m_needs_leading_return) {
-        m_needs_leading_return = false;
-        str += '\n';
-    }
-
-    str += fmt::format("{:%Y-%m-%d %H:%M:%S}  ",
+    *str += fmt::format("{:%Y-%m-%d %H:%M:%S}  ",
                        fmt::localtime(std::time(nullptr)));
 
     if (m_current_level == log_level::debug) {
-        str += fmt::format(ts, "[{:02d}] ", this_thread_num);
+        *str += fmt::format(ts, "[{:02d}] ", this_thread_num);
     }
 
     if (prefix) {
-        str += fmt::format(ts, "{}: ", prefix);
+        *str += fmt::format(ts, "{}: ", prefix);
     }
-
-    return str;
 }
 
 void logger::init_thread(unsigned int num)
diff --git a/src/logging.hpp b/src/logging.hpp
index 359a4ae7d..43a85224b 100644
--- a/src/logging.hpp
+++ b/src/logging.hpp
@@ -17,6 +17,7 @@
 #include <fmt/chrono.h>
 #include <fmt/color.h>
 
+#include <atomic>
 #include <cstdio>
 #include <utility>
 
@@ -45,7 +46,14 @@ class logger
 
         auto const &ts = m_use_color ? style : fmt::text_style{};
 
-        auto str = generate_common_prefix(ts, prefix);
+        std::string str;
+
+        if (m_needs_leading_return) {
+            m_needs_leading_return = false;
+            str += '\n';
+        }
+
+        generate_common_prefix(&str, ts, prefix);
 
         str += fmt::format(ts, format_str, std::forward<TArgs>(args)...);
         str += '\n';
@@ -82,14 +90,14 @@ class logger
     static void init_thread(unsigned int num);
 
 private:
-    std::string generate_common_prefix(fmt::text_style const &ts,
-                                       char const *prefix);
+    void generate_common_prefix(std::string *str, fmt::text_style const &ts,
+                                char const *prefix) const;
 
     log_level m_current_level = log_level::info;
     bool m_log_sql = false;
     bool m_log_sql_data = false;
     bool m_show_progress = true;
-    bool m_needs_leading_return = false;
+    std::atomic<bool> m_needs_leading_return = false;
 
 #ifdef _WIN32
     bool m_use_color = false;