LogManager: fix fp_ race between logging and SetFileLogPath/Shutdown

SetFileLogPath() and Shutdown() closed/reassigned fp_ without holding
logFileLock_, while LogLine() only locked around the actual fprintf,
after already reading fp_ unlocked. A concurrent SetFileLogPath() or
Shutdown() could close the FILE* mid-write from another thread. Now
all three paths hold logFileLock_ across the check-and-use of fp_.
Also fixes fp_ being left dangling (non-null but closed) if
SetFileLogPath() runs while File output is disabled.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
This commit is contained in:
Henrik RydgårdandClaude Sonnet 5 committed 2026-08-09 19:03:28 +02:00
1 parent 0f792defb4
commit 1c0e5ae48c
1 file changed
+10 -4
+10 -4
View File
@@ -146,9 +146,12 @@ void LogManager::Shutdown() {
return;
}
if (fp_) {
fclose(fp_);
fp_ = nullptr;
{
std::lock_guard<std::mutex> lk(logFileLock_);
if (fp_) {
fclose(fp_);
fp_ = nullptr;
}
}
outputs_ = (LogOutput)0;
@@ -193,6 +196,7 @@ LogManager::~LogManager() {
}
void LogManager::SetFileLogPath(const Path &filename) {
std::lock_guard<std::mutex> lk(logFileLock_);
if (fp_ && filename == logFilename_) {
// All good
return;
@@ -200,6 +204,7 @@ void LogManager::SetFileLogPath(const Path &filename) {
if (fp_) {
fclose(fp_);
fp_ = nullptr;
}
logFilename_ = Path(filename);
@@ -315,8 +320,9 @@ void LogManager::LogLine(LogLevel level, Log type, const char *file, int line, c
// OK, now go through the possible listeners in order.
if (outputs_ & LogOutput::File) {
// Lock covers the fp_ check too - SetFileLogPath()/Shutdown() can close it concurrently.
std::lock_guard<std::mutex> lk(logFileLock_);
if (fp_) {
std::lock_guard<std::mutex> lk(logFileLock_);
fprintf(fp_, "%s %s %s", message.timestamp, message.header, message.msg.c_str());
// Is this really necessary to do every time? I guess to catch the last message before a crash..
fflush(fp_);