From ee3374234c60aba2cc4c5cd5cac1c0aefc2d817c Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Tue, 1 Jan 2013 17:12:30 -0500 Subject: [PATCH] OutputDebugStringF code cleanup Initialize the OutputDebugStringF mutex and file pointer using boost::call_once, to be thread-safe. Make the return value of OutputDebugStringF really be the number of characters written (*printf() semantics). Declare the fReopenDebugLog flag volatile, since it is changed from a signal handler. And don't declare OutputDebugStringF() as inline. --- src/util.cpp | 97 ++++++++++++++++++++++++++++++---------------------- src/util.h | 2 +- 2 files changed, 57 insertions(+), 42 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index 576ba50db..d8f05cb9f 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -74,7 +74,7 @@ bool fTestNet = false; bool fNoListen = false; bool fLogTimestamps = false; CMedianFilter vTimeOffsets(200,0); -bool fReopenDebugLog = false; +volatile bool fReopenDebugLog = false; // Init OpenSSL library multithreading support static CCriticalSection** ppmutexOpenSSL; @@ -195,62 +195,76 @@ uint256 GetRandHash() +// +// OutputDebugStringF (aka printf -- there is a #define that we really +// should get rid of one day) 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). -inline int OutputDebugStringF(const char* pszFormat, ...) +static boost::once_flag debugPrintInitFlag = BOOST_ONCE_INIT; +// We use boost::call_once() to make sure these are initialized in +// in a thread-safe manner the first time it is called: +static FILE* fileout = NULL; +static boost::mutex* mutexDebugLog = NULL; + +static void DebugPrintInit() { - int ret = 0; + assert(fileout == NULL); + assert(mutexDebugLog == NULL); + + boost::filesystem::path pathDebug = GetDataDir() / "debug.log"; + fileout = fopen(pathDebug.string().c_str(), "a"); + if (fileout) setbuf(fileout, NULL); // unbuffered + + mutexDebugLog = new boost::mutex(); +} + +int OutputDebugStringF(const char* pszFormat, ...) +{ + int ret = 0; // Returns total number of characters written if (fPrintToConsole) { // print to console va_list arg_ptr; va_start(arg_ptr, pszFormat); - ret = vprintf(pszFormat, arg_ptr); + ret += vprintf(pszFormat, arg_ptr); va_end(arg_ptr); } else if (!fPrintToDebugger) { - // print to debug.log - static FILE* fileout = NULL; + static bool fStartedNewLine = true; + boost::call_once(&DebugPrintInit, debugPrintInitFlag); - if (!fileout) - { + if (fileout == NULL) + return ret; + + boost::mutex::scoped_lock scoped_lock(*mutexDebugLog); + + // reopen the log file, if requested + if (fReopenDebugLog) { + fReopenDebugLog = false; boost::filesystem::path pathDebug = GetDataDir() / "debug.log"; - fileout = fopen(pathDebug.string().c_str(), "a"); - if (fileout) setbuf(fileout, NULL); // unbuffered + if (freopen(pathDebug.string().c_str(),"a",fileout) != NULL) + setbuf(fileout, NULL); // unbuffered } - if (fileout) - { - static bool fStartedNewLine = true; - // This routine may be called by global destructors during shutdown. - // Since the order of destruction of static/global objects is undefined, - // allocate mutexDebugLog on the heap the first time this routine - // is called to avoid crashes during shutdown. - static boost::mutex* mutexDebugLog = NULL; - if (mutexDebugLog == NULL) mutexDebugLog = new boost::mutex(); - boost::mutex::scoped_lock scoped_lock(*mutexDebugLog); + // Debug print useful for profiling + if (fLogTimestamps && fStartedNewLine) + ret += fprintf(fileout, "%s ", DateTimeStrFormat("%Y-%m-%d %H:%M:%S", GetTime()).c_str()); + if (pszFormat[strlen(pszFormat) - 1] == '\n') + fStartedNewLine = true; + else + fStartedNewLine = false; - // reopen the log file, if requested - if (fReopenDebugLog) { - fReopenDebugLog = false; - boost::filesystem::path pathDebug = GetDataDir() / "debug.log"; - if (freopen(pathDebug.string().c_str(),"a",fileout) != NULL) - setbuf(fileout, NULL); // unbuffered - } - - // Debug print useful for profiling - if (fLogTimestamps && fStartedNewLine) - fprintf(fileout, "%s ", DateTimeStrFormat("%Y-%m-%d %H:%M:%S", GetTime()).c_str()); - if (pszFormat[strlen(pszFormat) - 1] == '\n') - fStartedNewLine = true; - else - fStartedNewLine = false; - - va_list arg_ptr; - va_start(arg_ptr, pszFormat); - ret = vfprintf(fileout, pszFormat, arg_ptr); - va_end(arg_ptr); - } + va_list arg_ptr; + va_start(arg_ptr, pszFormat); + ret += vfprintf(fileout, pszFormat, arg_ptr); + va_end(arg_ptr); } #ifdef WIN32 @@ -273,6 +287,7 @@ inline int OutputDebugStringF(const char* pszFormat, ...) { OutputDebugStringA(buffer.substr(line_start, line_end - line_start).c_str()); line_start = line_end + 1; + ret += line_end-line_start; } buffer.erase(0, line_start); } diff --git a/src/util.h b/src/util.h index 8bea0dd2b..97911d749 100644 --- a/src/util.h +++ b/src/util.h @@ -138,7 +138,7 @@ extern std::string strMiscWarning; extern bool fTestNet; extern bool fNoListen; extern bool fLogTimestamps; -extern bool fReopenDebugLog; +extern volatile bool fReopenDebugLog; void RandAddSeed(); void RandAddSeedPerfmon();