diff --git a/es-core/src/Log.cpp b/es-core/src/Log.cpp index 02b9468c4..b09e78049 100644 --- a/es-core/src/Log.cpp +++ b/es-core/src/Log.cpp @@ -9,8 +9,37 @@ #include "Log.h" #include "utils/StringUtil.h" +LogLevel Log::getReportingLevel() +{ + // Static Log functions need to grab the lock. + std::unique_lock lock {sLogMutex}; + return sReportingLevel; +} + +void Log::setReportingLevel(LogLevel level) +{ + // Static Log functions need to grab the lock. + std::unique_lock lock {sLogMutex}; + sReportingLevel = level; +} + +std::string Log::getLogPath() +{ + // No attempt is made to make this thread-safe. + // Currently getLogPath is public, and called in contexts with + // and without sLogMutex locked. + + // getHomePath() currently does not generate any Log messages. + return Utils::FileSystem::getHomePath() + "/.emulationstation/es_log.txt"; +} + void Log::init() { + // No attempt is made to make this thread-safe. + // It is unlikely to be called across multiple threads. + // Both removeFile and renameFile might generate log messages, + // so they might try to grab the lock. + Utils::FileSystem::removeFile(getLogPath() + ".bak"); // Rename the previous log file. Utils::FileSystem::renameFile(getLogPath(), getLogPath() + ".bak", true); @@ -19,6 +48,8 @@ void Log::init() void Log::open() { + // Static Log functions need to grab the lock. + std::unique_lock lock {sLogMutex}; #if defined(_WIN64) sFile.open(Utils::String::stringToWideString(getLogPath()).c_str()); #else @@ -28,6 +59,7 @@ void Log::open() std::ostringstream& Log::get(LogLevel level) { + // This function is not-static, lock is guarded by the Log() instance. time_t t {time(nullptr)}; struct tm tm; #if defined(_WIN64) @@ -45,18 +77,30 @@ std::ostringstream& Log::get(LogLevel level) void Log::flush() { - // Flush file. + // Flush file. Static Log functions need to grab the lock. + std::unique_lock lock {sLogMutex}; sFile.flush(); } void Log::close() { + // Static Log functions need to grab the lock. + std::unique_lock lock {sLogMutex}; if (sFile.is_open()) sFile.close(); } +Log::Log() +{ + // Log instance created. We grab the lock until destruction. + // This permits `Log().get(...) << msg << msg << msg;` to + // function as expected. + sLogMutex.lock(); +} + Log::~Log() { + // sLogMutex was (and still is) locked from the constructor Log(). mOutStringStream << std::endl; if (!sFile.is_open()) { @@ -72,4 +116,8 @@ Log::~Log() // If it's an error or the --debug flag has been set, then print to the console as well. if (mMessageLevel == LogError || sReportingLevel >= LogDebug) std::cerr << mOutStringStream.str(); + + // Release the lock, after any and all operations have been performed + // on mOutStringStream or sFile. + sLogMutex.unlock(); } diff --git a/es-core/src/Log.h b/es-core/src/Log.h index 3319fe771..887be03ff 100644 --- a/es-core/src/Log.h +++ b/es-core/src/Log.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #define LOG(level) \ @@ -33,18 +34,21 @@ enum LogLevel { class Log { public: + // Constructor/deconstructor handle a lock, making get() thread-safe. + Log(); ~Log(); std::ostringstream& get(LogLevel level = LogInfo); - static LogLevel getReportingLevel() { return sReportingLevel; } - static void setReportingLevel(LogLevel level) { sReportingLevel = level; } - static std::string getLogPath() - { - return Utils::FileSystem::getHomePath() + "/.emulationstation/es_log.txt"; - } + static LogLevel getReportingLevel(); + static void setReportingLevel(LogLevel level); - static void flush(); + // getLogPath() is not thread-safe. + static std::string getLogPath(); + // init() is not thread-safe. static void init(); + + // The following static functions are thread-safe. + static void flush(); static void open(); static void close(); @@ -59,6 +63,7 @@ private: {LogDebug, "Debug"}}; static inline std::ofstream sFile; static inline LogLevel sReportingLevel = LogInfo; + static inline std::recursive_mutex sLogMutex; LogLevel mMessageLevel; };