From 1577b96241cc60ca6eb47c9f4e0cb2f10ebf15a4 Mon Sep 17 00:00:00 2001 From: unknown <stewart@willster.(none)> Date: Fri, 3 Nov 2006 01:12:30 +1100 Subject: [PATCH] BUG#22309 FileLogHandler::createNewFile() isn't thread safe - may loose log messages BUG#22305 SysLogHandler not thread safe BUG#22313 can get duplicate log messages in cluster log Fix all these problems with one patch. Make Logger, hence EventLogger (with a bit more) thread safe. storage/ndb/include/debugger/EventLogger.hpp: remove m_text to make thread safe storage/ndb/include/logger/Logger.hpp: Use mutex to protect member variables for multithreaded use. storage/ndb/src/common/debugger/EventLogger.cpp: use a string on the stack instead of member variable to make class thread safe storage/ndb/src/common/logger/Logger.cpp: use mutexes to Guard member variables. makes class therad safe --- storage/ndb/include/debugger/EventLogger.hpp | 1 - storage/ndb/include/logger/Logger.hpp | 4 ++++ .../ndb/src/common/debugger/EventLogger.cpp | 21 ++++++++---------- storage/ndb/src/common/logger/Logger.cpp | 22 ++++++++++++++++++- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/storage/ndb/include/debugger/EventLogger.hpp b/storage/ndb/include/debugger/EventLogger.hpp index 6d09be70fe..044688c981 100644 --- a/storage/ndb/include/debugger/EventLogger.hpp +++ b/storage/ndb/include/debugger/EventLogger.hpp @@ -172,7 +172,6 @@ private: Uint32 m_filterLevel; STATIC_CONST(MAX_TEXT_LENGTH = 256); - char m_text[MAX_TEXT_LENGTH]; }; diff --git a/storage/ndb/include/logger/Logger.hpp b/storage/ndb/include/logger/Logger.hpp index 3414468d42..1589969281 100644 --- a/storage/ndb/include/logger/Logger.hpp +++ b/storage/ndb/include/logger/Logger.hpp @@ -276,6 +276,8 @@ public: protected: + NdbMutex *m_mutex; + void log(LoggerLevel logLevel, const char* msg, va_list ap) const; private: @@ -290,7 +292,9 @@ private: LogHandlerList* m_pHandlerList; const char* m_pCategory; + /* Default handlers */ + NdbMutex *m_handler_mutex; LogHandler* m_pConsoleHandler; LogHandler* m_pFileHandler; LogHandler* m_pSyslogHandler; diff --git a/storage/ndb/src/common/debugger/EventLogger.cpp b/storage/ndb/src/common/debugger/EventLogger.cpp index f1c6084754..a7a3ed970c 100644 --- a/storage/ndb/src/common/debugger/EventLogger.cpp +++ b/storage/ndb/src/common/debugger/EventLogger.cpp @@ -1004,6 +1004,7 @@ EventLogger::log(int eventType, const Uint32* theData, NodeId nodeId, Logger::LoggerLevel severity = Logger::LL_WARNING; LogLevel::EventCategory cat= LogLevel::llInvalid; EventTextFunction textF; + char log_text[MAX_TEXT_LENGTH]; DBUG_ENTER("EventLogger::log"); DBUG_PRINT("enter",("eventType=%d, nodeid=%d", eventType, nodeId)); @@ -1017,29 +1018,29 @@ EventLogger::log(int eventType, const Uint32* theData, NodeId nodeId, DBUG_PRINT("info",("m_logLevel.getLogLevel=%d", m_logLevel.getLogLevel(cat))); if (threshold <= set){ - getText(m_text,sizeof(m_text),textF,theData,nodeId); + getText(log_text,sizeof(log_text),textF,theData,nodeId); switch (severity){ case Logger::LL_ALERT: - alert(m_text); + alert(log_text); break; case Logger::LL_CRITICAL: - critical(m_text); + critical(log_text); break; case Logger::LL_WARNING: - warning(m_text); + warning(log_text); break; case Logger::LL_ERROR: - error(m_text); + error(log_text); break; case Logger::LL_INFO: - info(m_text); + info(log_text); break; case Logger::LL_DEBUG: - debug(m_text); + debug(log_text); break; default: - info(m_text); + info(log_text); break; } } // if (.. @@ -1057,7 +1058,3 @@ EventLogger::setFilterLevel(int filterLevel) { m_filterLevel = filterLevel; } - -// -// PRIVATE -// diff --git a/storage/ndb/src/common/logger/Logger.cpp b/storage/ndb/src/common/logger/Logger.cpp index 48e084a782..1715a6ee66 100644 --- a/storage/ndb/src/common/logger/Logger.cpp +++ b/storage/ndb/src/common/logger/Logger.cpp @@ -46,6 +46,8 @@ Logger::Logger() : m_pSyslogHandler(NULL) { m_pHandlerList = new LogHandlerList(); + m_mutex= NdbMutex_Create(); + m_handler_mutex= NdbMutex_Create(); disable(LL_ALL); enable(LL_ON); enable(LL_INFO); @@ -53,20 +55,25 @@ Logger::Logger() : Logger::~Logger() { - removeAllHandlers(); + removeAllHandlers(); delete m_pHandlerList; + NdbMutex_Destroy(m_handler_mutex); + NdbMutex_Destroy(m_mutex); } void Logger::setCategory(const char* pCategory) { + Guard g(m_mutex); m_pCategory = pCategory; } bool Logger::createConsoleHandler() { + Guard g(m_handler_mutex); bool rc = true; + if (m_pConsoleHandler == NULL) { m_pConsoleHandler = new ConsoleLogHandler(); @@ -84,6 +91,7 @@ Logger::createConsoleHandler() void Logger::removeConsoleHandler() { + Guard g(m_handler_mutex); if (removeHandler(m_pConsoleHandler)) { m_pConsoleHandler = NULL; @@ -93,6 +101,7 @@ Logger::removeConsoleHandler() bool Logger::createFileHandler() { + Guard g(m_handler_mutex); bool rc = true; if (m_pFileHandler == NULL) { @@ -111,6 +120,7 @@ Logger::createFileHandler() void Logger::removeFileHandler() { + Guard g(m_handler_mutex); if (removeHandler(m_pFileHandler)) { m_pFileHandler = NULL; @@ -120,6 +130,7 @@ Logger::removeFileHandler() bool Logger::createSyslogHandler() { + Guard g(m_handler_mutex); bool rc = true; if (m_pSyslogHandler == NULL) { @@ -142,6 +153,7 @@ Logger::createSyslogHandler() void Logger::removeSyslogHandler() { + Guard g(m_handler_mutex); if (removeHandler(m_pSyslogHandler)) { m_pSyslogHandler = NULL; @@ -151,6 +163,7 @@ Logger::removeSyslogHandler() bool Logger::addHandler(LogHandler* pHandler) { + Guard g(m_mutex); assert(pHandler != NULL); bool rc = pHandler->open(); @@ -224,6 +237,7 @@ Logger::addHandler(const BaseString &logstring, int *err, int len, char* errStr) bool Logger::removeHandler(LogHandler* pHandler) { + Guard g(m_mutex); int rc = false; if (pHandler != NULL) { @@ -236,12 +250,14 @@ Logger::removeHandler(LogHandler* pHandler) void Logger::removeAllHandlers() { + Guard g(m_mutex); m_pHandlerList->removeAll(); } bool Logger::isEnable(LoggerLevel logLevel) const { + Guard g(m_mutex); if (logLevel == LL_ALL) { for (unsigned i = 1; i < MAX_LOG_LEVELS; i++) @@ -255,6 +271,7 @@ Logger::isEnable(LoggerLevel logLevel) const void Logger::enable(LoggerLevel logLevel) { + Guard g(m_mutex); if (logLevel == LL_ALL) { for (unsigned i = 0; i < MAX_LOG_LEVELS; i++) @@ -271,6 +288,7 @@ Logger::enable(LoggerLevel logLevel) void Logger::enable(LoggerLevel fromLogLevel, LoggerLevel toLogLevel) { + Guard g(m_mutex); if (fromLogLevel > toLogLevel) { LoggerLevel tmp = toLogLevel; @@ -287,6 +305,7 @@ Logger::enable(LoggerLevel fromLogLevel, LoggerLevel toLogLevel) void Logger::disable(LoggerLevel logLevel) { + Guard g(m_mutex); if (logLevel == LL_ALL) { for (unsigned i = 0; i < MAX_LOG_LEVELS; i++) @@ -359,6 +378,7 @@ Logger::debug(const char* pMsg, ...) const void Logger::log(LoggerLevel logLevel, const char* pMsg, va_list ap) const { + Guard g(m_mutex); if (m_logLevels[LL_ON] && m_logLevels[logLevel]) { char buf[MAX_LOG_MESSAGE_SIZE]; -- 2.30.9