util: Move debug file management functions into Logger.

Author: Jim Posen

src/init.cpp

@@ -1235,10 +1235,11 @@ bool AppInitMain()
         if (gArgs.GetBoolArg("-shrinkdebugfile", logCategories == BCLog::NONE)) {
             // Do this first since it both loads a bunch of debug.log into memory,
             // and because this needs to happen before any other debug.log printing
-            ShrinkDebugFile();
+            g_logger->ShrinkDebugFile();
         }
-        if (!OpenDebugLog()) {
-            return InitError(strprintf("Could not open debug log file %s", GetDebugLogPath().string()));
+        if (!g_logger->OpenDebugLog()) {
+            return InitError(strprintf("Could not open debug log file %s",
+                                       g_logger->GetDebugLogPath().string()));
         }
     }
 

src/logging.cpp

@@ -7,9 +7,6 @@
 #include <util.h>
 #include <utilstrencodings.h>
 
-#include <list>
-#include <mutex>
-
 const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
 
 /**
@@ -31,57 +28,23 @@ bool fLogIPs = DEFAULT_LOGIPS;
 
 /** Log categories bitfield. */
 std::atomic<uint32_t> logCategories(0);
-/**
- * LogPrintf() has been broken a couple of times now
- * by well-meaning people adding mutexes in the most straightforward way.
- * It breaks because it may be called by global destructors during shutdown.
- * Since the order of destruction of static/global objects is undefined,
- * defining a mutex as a global object doesn't work (the mutex gets
- * destroyed, and then some later destructor calls OutputDebugStringF,
- * maybe indirectly, and you get a core dump at shutdown trying to lock
- * the mutex).
- */
-
-static std::once_flag debugPrintInitFlag;
-
-/**
- * We use std::call_once() to make sure mutexDebugLog and
- * vMsgsBeforeOpenLog are initialized in a thread-safe manner.
- *
- * NOTE: fileout, mutexDebugLog and sometimes vMsgsBeforeOpenLog
- * are leaked on exit. This is ugly, but will be cleaned up by
- * the OS/libc. When the shutdown sequence is fully audited and
- * tested, explicit destruction of these objects can be implemented.
- */
-static FILE* fileout = nullptr;
-static std::mutex* mutexDebugLog = nullptr;
-static std::list<std::string>* vMsgsBeforeOpenLog;
 
 static int FileWriteStr(const std::string &str, FILE *fp)
 {
     return fwrite(str.data(), 1, str.size(), fp);
 }
 
-static void DebugPrintInit()
-{
-    assert(mutexDebugLog == nullptr);
-    mutexDebugLog = new std::mutex();
-    vMsgsBeforeOpenLog = new std::list<std::string>;
-}
-
-fs::path GetDebugLogPath()
+fs::path BCLog::Logger::GetDebugLogPath() const
 {
     fs::path logfile(gArgs.GetArg("-debuglogfile", DEFAULT_DEBUGLOGFILE));
     return AbsPathForConfigVal(logfile);
 }
 
-bool OpenDebugLog()
+bool BCLog::Logger::OpenDebugLog()
 {
-    std::call_once(debugPrintInitFlag, &DebugPrintInit);
-    std::lock_guard<std::mutex> scoped_lock(*mutexDebugLog);
+    std::lock_guard<std::mutex> scoped_lock(mutexDebugLog);
 
     assert(fileout == nullptr);
-    assert(vMsgsBeforeOpenLog);
     fs::path pathDebug = GetDebugLogPath();
 
     fileout = fsbridge::fopen(pathDebug, "a");
@@ -91,13 +54,11 @@ bool OpenDebugLog()
 
     setbuf(fileout, nullptr); // unbuffered
     // dump buffered messages from before we opened the log
-    while (!vMsgsBeforeOpenLog->empty()) {
-        FileWriteStr(vMsgsBeforeOpenLog->front(), fileout);
-        vMsgsBeforeOpenLog->pop_front();
+    while (!vMsgsBeforeOpenLog.empty()) {
+        FileWriteStr(vMsgsBeforeOpenLog.front(), fileout);
+        vMsgsBeforeOpenLog.pop_front();
     }
 
-    delete vMsgsBeforeOpenLog;
-    vMsgsBeforeOpenLog = nullptr;
     return true;
 }
 
@@ -225,14 +186,12 @@ int BCLog::Logger::LogPrintStr(const std::string &str)
         fflush(stdout);
     }
     if (fPrintToDebugLog) {
-        std::call_once(debugPrintInitFlag, &DebugPrintInit);
-        std::lock_guard<std::mutex> scoped_lock(*mutexDebugLog);
+        std::lock_guard<std::mutex> scoped_lock(mutexDebugLog);
 
         // buffer if we haven't opened the log yet
         if (fileout == nullptr) {
-            assert(vMsgsBeforeOpenLog);
             ret = strTimestamped.length();
-            vMsgsBeforeOpenLog->push_back(strTimestamped);
+            vMsgsBeforeOpenLog.push_back(strTimestamped);
         }
         else
         {
@@ -250,7 +209,7 @@ int BCLog::Logger::LogPrintStr(const std::string &str)
     return ret;
 }
 
-void ShrinkDebugFile()
+void BCLog::Logger::ShrinkDebugFile()
 {
     // Amount of debug.log to save at end when shrinking (must fit in memory)
     constexpr size_t RECENT_DEBUG_HISTORY_SIZE = 10 * 1000000;

src/logging.h

@@ -11,6 +11,8 @@
 
 #include <atomic>
 #include <cstdint>
+#include <list>
+#include <mutex>
 #include <string>
 #include <vector>
 
@@ -59,6 +61,10 @@ namespace BCLog {
     class Logger
     {
     private:
+        FILE* fileout = nullptr;
+        std::mutex mutexDebugLog;
+        std::list<std::string> vMsgsBeforeOpenLog;
+
         /**
          * fStartedNewLine is a state variable that will suppress printing of
          * the timestamp when multiple calls are made that don't end in a
@@ -82,6 +88,10 @@ namespace BCLog {
 
         /** Returns whether logs will be written to any output */
         bool Enabled() const { return fPrintToConsole || fPrintToDebugLog; }
+
+        fs::path GetDebugLogPath() const;
+        bool OpenDebugLog();
+        void ShrinkDebugFile();
     };
 
 } // namespace BCLog
@@ -141,8 +151,4 @@ template<typename T, typename... Args> static inline void MarkUsed(const T& t, c
 } while(0)
 #endif
 
-fs::path GetDebugLogPath();
-bool OpenDebugLog();
-void ShrinkDebugFile();
-
 #endif // BITCOIN_LOGGING_H