From 89af6097f37587e178ac412252326351ca655075 Mon Sep 17 00:00:00 2001 From: jackpoz Date: Fri, 7 Feb 2014 23:24:24 +0100 Subject: Shared/Logs: Make AppenderFile more thread-safe In case of dynamic file names don't store the FILE* handle in a shared class variable but keep it only at function scope. Valgrind log: at _IO_un_link (genops.c:69) by fclose@@GLIBC_2.2.5 (iofclose.c:55) by AppenderFile::CloseFile() (AppenderFile.cpp:94) by AppenderFile::_write(LogMessage const&) (AppenderFile.cpp:66) by Appender::write(LogMessage&) (Appender.cpp:102) by Logger::write(LogMessage&) const (Logger.cpp:63) by Log::write(LogMessage*) (Log.cpp:279) by Log::vlog(std::string const&, LogLevel, char const*, __va_list_tag*) (Log.cpp:267) by Log::outMessage(std::string const&, LogLevel, char const*, ...) (Log.h:129) Address 0x2a1bd2d0 is 0 bytes inside a block of size 568 free'd at free (vg_replace_malloc.c:468) by fclose@@GLIBC_2.2.5 (iofclose.c:85) by AppenderFile::CloseFile() (AppenderFile.cpp:94) by AppenderFile::_write(LogMessage const&) (AppenderFile.cpp:66) by Appender::write(LogMessage&) (Appender.cpp:102) by Logger::write(LogMessage&) const (Logger.cpp:63) by Log::write(LogMessage*) (Log.cpp:279) by Log::vlog(std::string const&, LogLevel, char const*, __va_list_tag*) (Log.cpp:267) by Log::outMessage(std::string const&, LogLevel, char const*, ...) (Log.h:129) --- src/server/shared/Logging/AppenderFile.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'src/server/shared/Logging/AppenderFile.cpp') diff --git a/src/server/shared/Logging/AppenderFile.cpp b/src/server/shared/Logging/AppenderFile.cpp index 0d52de89f17..7544dc916df 100644 --- a/src/server/shared/Logging/AppenderFile.cpp +++ b/src/server/shared/Logging/AppenderFile.cpp @@ -33,7 +33,8 @@ AppenderFile::AppenderFile(uint8 id, std::string const& name, LogLevel level, co dynamicName = std::string::npos != filename.find("%s"); backup = (_flags & APPENDER_FLAGS_MAKE_FILE_BACKUP) != 0; - logfile = !dynamicName ? OpenFile(_filename, _mode, mode == "w" && backup) : NULL; + if (!dynamicName) + logfile = !dynamicName ? OpenFile(_filename, _mode, mode == "w" && backup) : NULL; } AppenderFile::~AppenderFile() @@ -50,7 +51,14 @@ void AppenderFile::_write(LogMessage const& message) char namebuf[TRINITY_PATH_MAX]; snprintf(namebuf, TRINITY_PATH_MAX, filename.c_str(), message.param1.c_str()); // always use "a" with dynamic name otherwise it could delete the log we wrote in last _write() call - logfile = OpenFile(namebuf, "a", backup || exceedMaxSize); + FILE* file = OpenFile(namebuf, "a", backup || exceedMaxSize); + if (!file) + return; + fprintf(file, "%s%s", message.prefix.c_str(), message.text.c_str()); + fflush(file); + fileSize += uint64(message.Size()); + fclose(file); + return; } else if (exceedMaxSize) logfile = OpenFile(filename, "w", true); @@ -61,9 +69,6 @@ void AppenderFile::_write(LogMessage const& message) fprintf(logfile, "%s%s", message.prefix.c_str(), message.text.c_str()); fflush(logfile); fileSize += uint64(message.Size()); - - if (dynamicName) - CloseFile(); } FILE* AppenderFile::OpenFile(std::string const &filename, std::string const &mode, bool backup) -- cgit v1.2.3