From 9578b5f87a4aa96e120b78f410ea90e141903dd7 Mon Sep 17 00:00:00 2001 From: jackpoz Date: Sat, 7 Sep 2013 14:37:54 +0200 Subject: Core/Thread: Fix race condition converting time values to local time Replace thread-unsafe localtime() http://www.cplusplus.com/reference/ctime/localtime/ with thread-safe portable ACE_OS::localtime_r() . Helgrind log: Possible data race during read of size 4 at 0x6F183C0 by thread #1 Locks held: none at 0x14E72E3: World::InitDailyQuestResetTime() (World.cpp:2772) by 0x14E3A01: World::SetInitialWorldSettings() (World.cpp:1790) by 0x101122A: Master::Run() (Master.cpp:164) by 0x101740C: main (Main.cpp:142) This conflicts with a previous write of size 4 by thread #2 Locks held: none at 0x6C2D3BA: __tzfile_compute (tzfile.c:797) by 0x6C2D036: __tz_convert (tzset.c:627) by 0x164146C: LogMessage::getTimeStr(long) (Appender.cpp:23) by 0x1641550: LogMessage::getTimeStr() (Appender.cpp:31) by 0x1641722: Appender::write(LogMessage&) (Appender.cpp:80) by 0x1633FCE: Logger::write(LogMessage&) (Logger.cpp:83) by 0x16433D8: LogOperation::call() (LogOperation.cpp:29) by 0x16428A4: LogWorker::svc() (LogWorker.cpp:45) --- src/server/shared/Logging/Appender.cpp | 5 +++-- src/server/shared/Logging/Log.cpp | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) (limited to 'src/server/shared/Logging') diff --git a/src/server/shared/Logging/Appender.cpp b/src/server/shared/Logging/Appender.cpp index cde94fe3a7c..f47cbc3b095 100644 --- a/src/server/shared/Logging/Appender.cpp +++ b/src/server/shared/Logging/Appender.cpp @@ -20,9 +20,10 @@ std::string LogMessage::getTimeStr(time_t time) { - tm* aTm = localtime(&time); + tm aTm; + ACE_OS::localtime_r(&time, &aTm); char buf[20]; - snprintf(buf, 20, "%04d-%02d-%02d_%02d:%02d:%02d", aTm->tm_year+1900, aTm->tm_mon+1, aTm->tm_mday, aTm->tm_hour, aTm->tm_min, aTm->tm_sec); + snprintf(buf, 20, "%04d-%02d-%02d_%02d:%02d:%02d", aTm.tm_year+1900, aTm.tm_mon+1, aTm.tm_mday, aTm.tm_hour, aTm.tm_min, aTm.tm_sec); return std::string(buf); } diff --git a/src/server/shared/Logging/Log.cpp b/src/server/shared/Logging/Log.cpp index be7aab9cf5e..e6400176155 100644 --- a/src/server/shared/Logging/Log.cpp +++ b/src/server/shared/Logging/Log.cpp @@ -295,7 +295,8 @@ void Log::write(LogMessage* msg) std::string Log::GetTimestampStr() { time_t t = time(NULL); - tm* aTm = localtime(&t); + tm aTm; + ACE_OS::localtime_r(&t, &aTm); // YYYY year // MM month (2 digits 01-12) // DD day (2 digits 01-31) @@ -303,7 +304,7 @@ std::string Log::GetTimestampStr() // MM minutes (2 digits 00-59) // SS seconds (2 digits 00-59) char buf[20]; - snprintf(buf, 20, "%04d-%02d-%02d_%02d-%02d-%02d", aTm->tm_year+1900, aTm->tm_mon+1, aTm->tm_mday, aTm->tm_hour, aTm->tm_min, aTm->tm_sec); + snprintf(buf, 20, "%04d-%02d-%02d_%02d-%02d-%02d", aTm.tm_year+1900, aTm.tm_mon+1, aTm.tm_mday, aTm.tm_hour, aTm.tm_min, aTm.tm_sec); return std::string(buf); } -- cgit v1.2.3 From 1b3575ba772124d2e7a8173a5d35dc9d2d271a8a Mon Sep 17 00:00:00 2001 From: jackpoz Date: Thu, 12 Sep 2013 22:54:05 +0200 Subject: Core/Logs: Fix race condition in Log Fix race condition in Log by using atomic operators. Helgrind log: Possible data race during read of size 8 at 0x7379D98 by thread #1 Locks held: none at 0x15AE9C7: AppenderFile::_write(LogMessage const&) (AppenderFile.cpp:59) by 0x15ADFF8: Appender::write(LogMessage&) (Appender.cpp:106) by 0x159F14E: Logger::write(LogMessage&) (Logger.cpp:83) by 0x15A215B: Log::write(LogMessage*) (Log.cpp:290) by 0x15A200F: Log::vlog(LogFilterType, LogLevel, char const*, __va_list_tag*) (Log.cpp:272) by 0x15A2682: Log::outInfo(LogFilterType, char const*, ...) (Log.cpp:364) by 0xF7DA28: Master::Run() (Master.cpp:296) by 0xF835E8: main (Main.cpp:142) This conflicts with a previous write of size 8 by thread #10 Locks held: none at 0x15AE9D7: AppenderFile::_write(LogMessage const&) (AppenderFile.cpp:59) by 0x15ADFF8: Appender::write(LogMessage&) (Appender.cpp:106) by 0x159F14E: Logger::write(LogMessage&) (Logger.cpp:83) by 0x15A215B: Log::write(LogMessage*) (Log.cpp:290) by 0x15A200F: Log::vlog(LogFilterType, LogLevel, char const*, __va_list_tag*) (Log.cpp:272) by 0x15A2682: Log::outInfo(LogFilterType, char const*, ...) (Log.cpp:364) by 0xF7EC1F: FreezeDetectorRunnable::run() (Master.cpp:98) by 0x15A5B3E: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186) Address 0x7379D98 is 88 bytes inside a block of size 96 alloc'd at 0x4C2C857: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so) by 0x15A0FE3: Log::CreateAppenderFromConfig(char const*) (Log.cpp:150) by 0x15A1AED: Log::ReadAppendersFromConfig() (Log.cpp:244) by 0x15A31B4: Log::LoadFromConfig() (Log.cpp:469) by 0x15A053B: Log::Log() (Log.cpp:35) by 0xF75CD1: ACE_Singleton::ACE_Singleton() (Singleton.inl:13) by 0xF754A4: ACE_Singleton::instance() (Singleton.cpp:91) by 0xF8351A: main (Main.cpp:135) --- src/server/shared/Logging/AppenderFile.cpp | 4 ++-- src/server/shared/Logging/AppenderFile.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'src/server/shared/Logging') diff --git a/src/server/shared/Logging/AppenderFile.cpp b/src/server/shared/Logging/AppenderFile.cpp index 93d53bcc30d..54458346bb9 100644 --- a/src/server/shared/Logging/AppenderFile.cpp +++ b/src/server/shared/Logging/AppenderFile.cpp @@ -40,7 +40,7 @@ AppenderFile::~AppenderFile() void AppenderFile::_write(LogMessage const& message) { - bool exceedMaxSize = maxFileSize > 0 && (fileSize + message.Size()) > maxFileSize; + bool exceedMaxSize = maxFileSize > 0 && (fileSize.value() + message.Size()) > maxFileSize; if (dynamicName) { @@ -56,7 +56,7 @@ void AppenderFile::_write(LogMessage const& message) fprintf(logfile, "%s%s", message.prefix.c_str(), message.text.c_str()); fflush(logfile); - fileSize += message.Size(); + fileSize += uint64(message.Size()); if (dynamicName) CloseFile(); diff --git a/src/server/shared/Logging/AppenderFile.h b/src/server/shared/Logging/AppenderFile.h index c15974799e1..de94a46d692 100644 --- a/src/server/shared/Logging/AppenderFile.h +++ b/src/server/shared/Logging/AppenderFile.h @@ -19,6 +19,7 @@ #define APPENDERFILE_H #include "Appender.h" +#include "ace/Atomic_Op.h" class AppenderFile: public Appender { @@ -37,7 +38,7 @@ class AppenderFile: public Appender bool dynamicName; bool backup; uint64 maxFileSize; - uint64 fileSize; + ACE_Atomic_Op fileSize; }; #endif -- cgit v1.2.3