From bf1954b38918c8e09824b574df4e88527457d004 Mon Sep 17 00:00:00 2001
From: Dennis Felsing <dennis@felsin9.de>
Date: Thu, 18 May 2023 18:01:21 +0200
Subject: [PATCH] Fix logging (fixes #6611)

@heinrich5991 Was the atomic important? Could we have a lock instead if so?

Seems a bit annoying since atomic can't have a shared_ptr inside
---
 src/base/log.cpp             | 16 +++++++++-------
 src/base/logger.h            |  4 ++--
 src/engine/shared/engine.cpp |  4 ++--
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/base/log.cpp b/src/base/log.cpp
index 6814ab6a3..96f53cf36 100644
--- a/src/base/log.cpp
+++ b/src/base/log.cpp
@@ -5,6 +5,7 @@
 
 #include <atomic>
 #include <cstdio>
+#include <memory>
 
 #if defined(CONF_FAMILY_WINDOWS)
 #define WIN32_LEAN_AND_MEAN
@@ -423,18 +424,19 @@ std::unique_ptr<ILogger> log_logger_windows_debugger()
 }
 #endif
 
-void CFutureLogger::Set(std::unique_ptr<ILogger> &&pLogger)
+void CFutureLogger::Set(std::shared_ptr<ILogger> pLogger)
 {
-	ILogger *null = nullptr;
+	std::shared_ptr<ILogger> null;
 	m_PendingLock.lock();
-	ILogger *pLoggerRaw = pLogger.release();
-	if(!m_pLogger.compare_exchange_strong(null, pLoggerRaw, std::memory_order_acq_rel))
+	if(!std::atomic_compare_exchange_strong_explicit(&m_pLogger, &null, pLogger, std::memory_order_acq_rel, std::memory_order_acq_rel))
 	{
 		dbg_assert(false, "future logger has already been set and can only be set once");
 	}
+	m_pLogger = std::move(pLogger);
+
 	for(const auto &Pending : m_vPending)
 	{
-		pLoggerRaw->Log(&Pending);
+		m_pLogger->Log(&Pending);
 	}
 	m_vPending.clear();
 	m_vPending.shrink_to_fit();
@@ -443,7 +445,7 @@ void CFutureLogger::Set(std::unique_ptr<ILogger> &&pLogger)
 
 void CFutureLogger::Log(const CLogMessage *pMessage)
 {
-	ILogger *pLogger = m_pLogger.load(std::memory_order_acquire);
+	auto pLogger = std::atomic_load_explicit(&m_pLogger, std::memory_order_acquire);
 	if(pLogger)
 	{
 		pLogger->Log(pMessage);
@@ -456,7 +458,7 @@ void CFutureLogger::Log(const CLogMessage *pMessage)
 
 void CFutureLogger::GlobalFinish()
 {
-	ILogger *pLogger = m_pLogger.load(std::memory_order_acquire);
+	auto pLogger = std::atomic_load_explicit(&m_pLogger, std::memory_order_acquire);
 	if(pLogger)
 	{
 		pLogger->GlobalFinish();
diff --git a/src/base/logger.h b/src/base/logger.h
index 618a1d051..9624c8c12 100644
--- a/src/base/logger.h
+++ b/src/base/logger.h
@@ -202,7 +202,7 @@ std::unique_ptr<ILogger> log_logger_windows_debugger();
 class CFutureLogger : public ILogger
 {
 private:
-	std::atomic<ILogger *> m_pLogger;
+	std::shared_ptr<ILogger> m_pLogger;
 	std::vector<CLogMessage> m_vPending;
 	std::mutex m_PendingLock;
 
@@ -211,7 +211,7 @@ public:
 	 * Replace the `CFutureLogger` instance with the given logger. It'll
 	 * receive all log messages sent to the `CFutureLogger` so far.
 	 */
-	void Set(std::unique_ptr<ILogger> &&pLogger);
+	void Set(std::shared_ptr<ILogger> pLogger);
 	void Log(const CLogMessage *pMessage) override;
 	void GlobalFinish() override;
 };
diff --git a/src/engine/shared/engine.cpp b/src/engine/shared/engine.cpp
index c46dc9fac..395cf7853 100644
--- a/src/engine/shared/engine.cpp
+++ b/src/engine/shared/engine.cpp
@@ -30,7 +30,7 @@ public:
 	IStorage *m_pStorage;
 	bool m_Logging;
 
-	std::shared_ptr<ILogger> m_pFutureLogger;
+	std::shared_ptr<CFutureLogger> m_pFutureLogger;
 
 	char m_aAppName[256];
 
@@ -115,7 +115,7 @@ public:
 
 	void SetAdditionalLogger(std::shared_ptr<ILogger> &&pLogger) override
 	{
-		m_pFutureLogger = pLogger;
+		m_pFutureLogger->Set(pLogger);
 	}
 };