From 15f14806a80205bf94b26dc22e57435e692dac3e Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 4 Feb 2019 14:26:02 -0500 Subject: [PATCH] Merge #15266: memory: Construct globals on first use 77777c5624 log: Construct global logger on first use (MarcoFalke) Pull request description: The (de)initialization order is not well defined in C++, so generally it is not safe to use globals as the (de/con)structor of one global could use the (de/con)structor of another global before/after it has been (con/de)structed. Specifically this fixes: * `g_logger` might not be initialized on the first use, so do that. (Fixes #15111) Tree-SHA512: eb9c22f4baf31ebc5b0b9ee6a51d1354bae1f0df186cc0ce818b4483c7b5a7f90268d2b549ee96b4c57f8ef36ab239dc6497f74f3e2ef166038f7437c368297d --- src/batchedlogger.cpp | 2 +- src/httpserver.cpp | 4 ++-- src/init.cpp | 30 +++++++++++++++--------------- src/interfaces/node.cpp | 2 +- src/llmq/quorums_commitment.cpp | 2 +- src/logging.cpp | 12 +++++++++--- src/logging.h | 8 ++++---- src/rpc/misc.cpp | 16 ++++++++-------- 8 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/batchedlogger.cpp b/src/batchedlogger.cpp index 55aaa7fec6..cf07b87222 100644 --- a/src/batchedlogger.cpp +++ b/src/batchedlogger.cpp @@ -20,6 +20,6 @@ void CBatchedLogger::Flush() if (!accept || msg.empty()) { return; } - g_logger->LogPrintStr(strprintf("%s:\n%s", header, msg)); + LogInstance().LogPrintStr(strprintf("%s:\n%s", header, msg)); msg.clear(); } diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 34947f59f9..013fe0040d 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -373,8 +373,8 @@ bool InitHTTPServer() // Update libevent's log handling. Returns false if our version of // libevent doesn't support debug logging, in which case we should // clear the BCLog::LIBEVENT flag. - if (!UpdateHTTPServerLogging(g_logger->WillLogCategory(BCLog::LIBEVENT))) { - g_logger->DisableCategory(BCLog::LIBEVENT); + if (!UpdateHTTPServerLogging(LogInstance().WillLogCategory(BCLog::LIBEVENT))) { + LogInstance().DisableCategory(BCLog::LIBEVENT); } #ifdef WIN32 diff --git a/src/init.cpp b/src/init.cpp index 87a047f2dd..4c1030fa0a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -416,7 +416,7 @@ static void HandleSIGTERM(int) static void HandleSIGHUP(int) { - g_logger->m_reopen_file = true; + LogInstance().m_reopen_file = true; } #else static BOOL WINAPI consoleCtrlHandler(DWORD dwCtrlType) @@ -1185,12 +1185,12 @@ static std::string ResolveErrMsg(const char * const optname, const std::string& */ void InitLogging() { - g_logger->m_print_to_file = !gArgs.IsArgNegated("-debuglogfile"); - g_logger->m_file_path = AbsPathForConfigVal(gArgs.GetArg("-debuglogfile", DEFAULT_DEBUGLOGFILE)); - g_logger->m_print_to_console = gArgs.GetBoolArg("-printtoconsole", !gArgs.GetBoolArg("-daemon", false)); - g_logger->m_log_timestamps = gArgs.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS); - g_logger->m_log_time_micros = gArgs.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS); - g_logger->m_log_threadnames = gArgs.GetBoolArg("-logthreadnames", DEFAULT_LOGTHREADNAMES); + LogInstance().m_print_to_file = !gArgs.IsArgNegated("-debuglogfile"); + LogInstance().m_file_path = AbsPathForConfigVal(gArgs.GetArg("-debuglogfile", DEFAULT_DEBUGLOGFILE)); + LogInstance().m_print_to_console = gArgs.GetBoolArg("-printtoconsole", !gArgs.GetBoolArg("-daemon", false)); + LogInstance().m_log_timestamps = gArgs.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS); + LogInstance().m_log_time_micros = gArgs.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS); + LogInstance().m_log_threadnames = gArgs.GetBoolArg("-logthreadnames", DEFAULT_LOGTHREADNAMES); fLogIPs = gArgs.GetBoolArg("-logips", DEFAULT_LOGIPS); @@ -1366,7 +1366,7 @@ bool AppInitParameterInteraction() if (std::none_of(categories.begin(), categories.end(), [](std::string cat){return cat == "0" || cat == "none";})) { for (const auto& cat : categories) { - if (!g_logger->EnableCategory(cat)) { + if (!LogInstance().EnableCategory(cat)) { InitWarning(strprintf(_("Unsupported logging category %s=%s."), "-debug", cat)); } } @@ -1375,7 +1375,7 @@ bool AppInitParameterInteraction() // Now remove the logging categories which were explicitly excluded for (const std::string& cat : gArgs.GetArgs("-debugexclude")) { - if (!g_logger->DisableCategory(cat)) { + if (!LogInstance().DisableCategory(cat)) { InitWarning(strprintf(_("Unsupported logging category %s=%s."), "-debugexclude", cat)); } } @@ -1619,19 +1619,19 @@ bool AppInitMain() return false; } #endif - if (g_logger->m_print_to_file) { - if (gArgs.GetBoolArg("-shrinkdebugfile", g_logger->DefaultShrinkDebugFile())) { + if (LogInstance().m_print_to_file) { + if (gArgs.GetBoolArg("-shrinkdebugfile", LogInstance().DefaultShrinkDebugFile())) { // 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 - g_logger->ShrinkDebugFile(); + LogInstance().ShrinkDebugFile(); } } - if (!g_logger->StartLogging()) { + if (!LogInstance().StartLogging()) { return InitError(strprintf("Could not open debug log file %s", - g_logger->m_file_path.string())); + LogInstance().m_file_path.string())); } - if (!g_logger->m_log_timestamps) + if (!LogInstance().m_log_timestamps) LogPrintf("Startup time: %s\n", FormatISO8601DateTime(GetTime())); LogPrintf("Default data directory %s\n", GetDefaultDataDir().string()); LogPrintf("Using data directory %s\n", GetDataDir().string()); diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index b1ffa68abe..c7b3f7d4ac 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -175,7 +175,7 @@ class NodeImpl : public Node void initLogging() override { InitLogging(); } void initParameterInteraction() override { InitParameterInteraction(); } std::string getWarnings(const std::string& type) override { return GetWarnings(type); } - uint64_t getLogCategories() override { return g_logger->GetCategoryMask(); } + uint64_t getLogCategories() override { return LogInstance().GetCategoryMask(); } bool baseInitialize() override { return AppInitBasicSetup() && AppInitParameterInteraction() && AppInitSanityChecks() && diff --git a/src/llmq/quorums_commitment.cpp b/src/llmq/quorums_commitment.cpp index 73d6517422..3b1005c3eb 100644 --- a/src/llmq/quorums_commitment.cpp +++ b/src/llmq/quorums_commitment.cpp @@ -24,7 +24,7 @@ CFinalCommitment::CFinalCommitment(const Consensus::LLMQParams& params, const ui } #define LogPrintfFinalCommitment(...) do { \ - g_logger->LogPrintStr(strprintf("CFinalCommitment::%s -- %s", __func__, tinyformat::format(__VA_ARGS__))); \ + LogInstance().LogPrintStr(strprintf("CFinalCommitment::%s -- %s", __func__, tinyformat::format(__VA_ARGS__))); \ } while(0) bool CFinalCommitment::Verify(const CBlockIndex* pQuorumIndex, bool checkSigs) const diff --git a/src/logging.cpp b/src/logging.cpp index 7bb1807ac0..6bc00749c4 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -12,6 +12,8 @@ const char * const DEFAULT_DEBUGLOGFILE = "debug.log"; +BCLog::Logger& LogInstance() +{ /** * NOTE: the logger instances is leaked on exit. This is ugly, but will be * cleaned up by the OS/libc. Defining a logger as a global object doesn't work @@ -21,11 +23,15 @@ const char * const DEFAULT_DEBUGLOGFILE = "debug.log"; * access the logger. When the shutdown sequence is fully audited and tested, * explicit destruction of these objects can be implemented by changing this * from a raw pointer to a std::unique_ptr. + * Since the destructor is never called, the logger and all its members must + * have a trivial destructor. * * This method of initialization was originally introduced in * ee3374234c60aba2cc4c5cd5cac1c0aefc2d817c. */ -BCLog::Logger* const g_logger = new BCLog::Logger(); + static BCLog::Logger* g_logger{new BCLog::Logger()}; + return *g_logger; +} bool fLogIPs = DEFAULT_LOGIPS; @@ -204,9 +210,9 @@ std::vector ListActiveLogCategories() std::string ListActiveLogCategoriesString() { - if (g_logger->GetCategoryMask() == BCLog::NONE) + if (LogInstance().GetCategoryMask() == BCLog::NONE) return "0"; - if (g_logger->GetCategoryMask() == BCLog::ALL) + if (LogInstance().GetCategoryMask() == BCLog::ALL) return "1"; std::string ret; diff --git a/src/logging.h b/src/logging.h index 4de98e6df5..227b44a166 100644 --- a/src/logging.h +++ b/src/logging.h @@ -142,12 +142,12 @@ namespace BCLog { } // namespace BCLog -extern BCLog::Logger* const g_logger; +BCLog::Logger& LogInstance(); /** Return true if log accepts specified category */ static inline bool LogAcceptCategory(BCLog::LogFlags category) { - return g_logger->WillLogCategory(category); + return LogInstance().WillLogCategory(category); } /** Returns a string with the log categories. */ @@ -182,7 +182,7 @@ std::string SafeStringFormat(const std::string& fmt, const Args&... args) template static inline void LogPrintf(const char* fmt, const Args&... args) { - if (g_logger->Enabled()) { + if (LogInstance().Enabled()) { std::string log_msg; try { log_msg = tfm::format(fmt, args...); @@ -190,7 +190,7 @@ static inline void LogPrintf(const char* fmt, const Args&... args) /* Original format string will have newline so don't add one here */ log_msg = "Error \"" + std::string(fmterr.what()) + "\" while formatting log message: " + fmt; } - g_logger->LogPrintStr(log_msg); + LogInstance().LogPrintStr(log_msg); } } diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 650a6fc24c..109dddb2ed 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -59,14 +59,14 @@ static UniValue debug(const JSONRPCRequest& request) ); std::string strMode = request.params[0].get_str(); - g_logger->DisableCategory(BCLog::ALL); + LogInstance().DisableCategory(BCLog::ALL); std::vector categories; boost::split(categories, strMode, boost::is_any_of("+")); if (std::find(categories.begin(), categories.end(), std::string("0")) == categories.end()) { for (const auto& cat : categories) { - g_logger->EnableCategory(cat); + LogInstance().EnableCategory(cat); } } @@ -1023,9 +1023,9 @@ static void EnableOrDisableLogCategories(UniValue cats, bool enable) { bool success; if (enable) { - success = g_logger->EnableCategory(cat); + success = LogInstance().EnableCategory(cat); } else { - success = g_logger->DisableCategory(cat); + success = LogInstance().DisableCategory(cat); } if (!success) { @@ -1072,14 +1072,14 @@ static UniValue logging(const JSONRPCRequest& request) ); } - uint64_t original_log_categories = g_logger->GetCategoryMask(); + uint64_t original_log_categories = LogInstance().GetCategoryMask(); if (request.params[0].isArray()) { EnableOrDisableLogCategories(request.params[0], true); } if (request.params[1].isArray()) { EnableOrDisableLogCategories(request.params[1], false); } - uint64_t updated_log_categories = g_logger->GetCategoryMask(); + uint64_t updated_log_categories = LogInstance().GetCategoryMask(); uint64_t changed_log_categories = original_log_categories ^ updated_log_categories; // Update libevent logging if BCLog::LIBEVENT has changed. @@ -1088,8 +1088,8 @@ static UniValue logging(const JSONRPCRequest& request) // Throw an error if the user has explicitly asked to change only the libevent // flag and it failed. if (changed_log_categories & BCLog::LIBEVENT) { - if (!UpdateHTTPServerLogging(g_logger->WillLogCategory(BCLog::LIBEVENT))) { - g_logger->DisableCategory(BCLog::LIBEVENT); + if (!UpdateHTTPServerLogging(LogInstance().WillLogCategory(BCLog::LIBEVENT))) { + LogInstance().DisableCategory(BCLog::LIBEVENT); if (changed_log_categories == BCLog::LIBEVENT) { throw JSONRPCError(RPC_INVALID_PARAMETER, "libevent logging cannot be updated when using libevent before v2.1.1."); }