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.
parent d345a1fd
...@@ -172,7 +172,6 @@ private: ...@@ -172,7 +172,6 @@ private:
Uint32 m_filterLevel; Uint32 m_filterLevel;
STATIC_CONST(MAX_TEXT_LENGTH = 256); STATIC_CONST(MAX_TEXT_LENGTH = 256);
char m_text[MAX_TEXT_LENGTH];
}; };
......
...@@ -276,6 +276,8 @@ public: ...@@ -276,6 +276,8 @@ public:
protected: protected:
NdbMutex *m_mutex;
void log(LoggerLevel logLevel, const char* msg, va_list ap) const; void log(LoggerLevel logLevel, const char* msg, va_list ap) const;
private: private:
...@@ -290,7 +292,9 @@ private: ...@@ -290,7 +292,9 @@ private:
LogHandlerList* m_pHandlerList; LogHandlerList* m_pHandlerList;
const char* m_pCategory; const char* m_pCategory;
/* Default handlers */ /* Default handlers */
NdbMutex *m_handler_mutex;
LogHandler* m_pConsoleHandler; LogHandler* m_pConsoleHandler;
LogHandler* m_pFileHandler; LogHandler* m_pFileHandler;
LogHandler* m_pSyslogHandler; LogHandler* m_pSyslogHandler;
......
...@@ -1004,6 +1004,7 @@ EventLogger::log(int eventType, const Uint32* theData, NodeId nodeId, ...@@ -1004,6 +1004,7 @@ EventLogger::log(int eventType, const Uint32* theData, NodeId nodeId,
Logger::LoggerLevel severity = Logger::LL_WARNING; Logger::LoggerLevel severity = Logger::LL_WARNING;
LogLevel::EventCategory cat= LogLevel::llInvalid; LogLevel::EventCategory cat= LogLevel::llInvalid;
EventTextFunction textF; EventTextFunction textF;
char log_text[MAX_TEXT_LENGTH];
DBUG_ENTER("EventLogger::log"); DBUG_ENTER("EventLogger::log");
DBUG_PRINT("enter",("eventType=%d, nodeid=%d", eventType, nodeId)); DBUG_PRINT("enter",("eventType=%d, nodeid=%d", eventType, nodeId));
...@@ -1017,29 +1018,29 @@ EventLogger::log(int eventType, const Uint32* theData, NodeId 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))); DBUG_PRINT("info",("m_logLevel.getLogLevel=%d", m_logLevel.getLogLevel(cat)));
if (threshold <= set){ if (threshold <= set){
getText(m_text,sizeof(m_text),textF,theData,nodeId); getText(log_text,sizeof(log_text),textF,theData,nodeId);
switch (severity){ switch (severity){
case Logger::LL_ALERT: case Logger::LL_ALERT:
alert(m_text); alert(log_text);
break; break;
case Logger::LL_CRITICAL: case Logger::LL_CRITICAL:
critical(m_text); critical(log_text);
break; break;
case Logger::LL_WARNING: case Logger::LL_WARNING:
warning(m_text); warning(log_text);
break; break;
case Logger::LL_ERROR: case Logger::LL_ERROR:
error(m_text); error(log_text);
break; break;
case Logger::LL_INFO: case Logger::LL_INFO:
info(m_text); info(log_text);
break; break;
case Logger::LL_DEBUG: case Logger::LL_DEBUG:
debug(m_text); debug(log_text);
break; break;
default: default:
info(m_text); info(log_text);
break; break;
} }
} // if (.. } // if (..
...@@ -1057,7 +1058,3 @@ EventLogger::setFilterLevel(int filterLevel) ...@@ -1057,7 +1058,3 @@ EventLogger::setFilterLevel(int filterLevel)
{ {
m_filterLevel = filterLevel; m_filterLevel = filterLevel;
} }
//
// PRIVATE
//
...@@ -46,6 +46,8 @@ Logger::Logger() : ...@@ -46,6 +46,8 @@ Logger::Logger() :
m_pSyslogHandler(NULL) m_pSyslogHandler(NULL)
{ {
m_pHandlerList = new LogHandlerList(); m_pHandlerList = new LogHandlerList();
m_mutex= NdbMutex_Create();
m_handler_mutex= NdbMutex_Create();
disable(LL_ALL); disable(LL_ALL);
enable(LL_ON); enable(LL_ON);
enable(LL_INFO); enable(LL_INFO);
...@@ -53,20 +55,25 @@ Logger::Logger() : ...@@ -53,20 +55,25 @@ Logger::Logger() :
Logger::~Logger() Logger::~Logger()
{ {
removeAllHandlers(); removeAllHandlers();
delete m_pHandlerList; delete m_pHandlerList;
NdbMutex_Destroy(m_handler_mutex);
NdbMutex_Destroy(m_mutex);
} }
void void
Logger::setCategory(const char* pCategory) Logger::setCategory(const char* pCategory)
{ {
Guard g(m_mutex);
m_pCategory = pCategory; m_pCategory = pCategory;
} }
bool bool
Logger::createConsoleHandler() Logger::createConsoleHandler()
{ {
Guard g(m_handler_mutex);
bool rc = true; bool rc = true;
if (m_pConsoleHandler == NULL) if (m_pConsoleHandler == NULL)
{ {
m_pConsoleHandler = new ConsoleLogHandler(); m_pConsoleHandler = new ConsoleLogHandler();
...@@ -84,6 +91,7 @@ Logger::createConsoleHandler() ...@@ -84,6 +91,7 @@ Logger::createConsoleHandler()
void void
Logger::removeConsoleHandler() Logger::removeConsoleHandler()
{ {
Guard g(m_handler_mutex);
if (removeHandler(m_pConsoleHandler)) if (removeHandler(m_pConsoleHandler))
{ {
m_pConsoleHandler = NULL; m_pConsoleHandler = NULL;
...@@ -93,6 +101,7 @@ Logger::removeConsoleHandler() ...@@ -93,6 +101,7 @@ Logger::removeConsoleHandler()
bool bool
Logger::createFileHandler() Logger::createFileHandler()
{ {
Guard g(m_handler_mutex);
bool rc = true; bool rc = true;
if (m_pFileHandler == NULL) if (m_pFileHandler == NULL)
{ {
...@@ -111,6 +120,7 @@ Logger::createFileHandler() ...@@ -111,6 +120,7 @@ Logger::createFileHandler()
void void
Logger::removeFileHandler() Logger::removeFileHandler()
{ {
Guard g(m_handler_mutex);
if (removeHandler(m_pFileHandler)) if (removeHandler(m_pFileHandler))
{ {
m_pFileHandler = NULL; m_pFileHandler = NULL;
...@@ -120,6 +130,7 @@ Logger::removeFileHandler() ...@@ -120,6 +130,7 @@ Logger::removeFileHandler()
bool bool
Logger::createSyslogHandler() Logger::createSyslogHandler()
{ {
Guard g(m_handler_mutex);
bool rc = true; bool rc = true;
if (m_pSyslogHandler == NULL) if (m_pSyslogHandler == NULL)
{ {
...@@ -142,6 +153,7 @@ Logger::createSyslogHandler() ...@@ -142,6 +153,7 @@ Logger::createSyslogHandler()
void void
Logger::removeSyslogHandler() Logger::removeSyslogHandler()
{ {
Guard g(m_handler_mutex);
if (removeHandler(m_pSyslogHandler)) if (removeHandler(m_pSyslogHandler))
{ {
m_pSyslogHandler = NULL; m_pSyslogHandler = NULL;
...@@ -151,6 +163,7 @@ Logger::removeSyslogHandler() ...@@ -151,6 +163,7 @@ Logger::removeSyslogHandler()
bool bool
Logger::addHandler(LogHandler* pHandler) Logger::addHandler(LogHandler* pHandler)
{ {
Guard g(m_mutex);
assert(pHandler != NULL); assert(pHandler != NULL);
bool rc = pHandler->open(); bool rc = pHandler->open();
...@@ -224,6 +237,7 @@ Logger::addHandler(const BaseString &logstring, int *err, int len, char* errStr) ...@@ -224,6 +237,7 @@ Logger::addHandler(const BaseString &logstring, int *err, int len, char* errStr)
bool bool
Logger::removeHandler(LogHandler* pHandler) Logger::removeHandler(LogHandler* pHandler)
{ {
Guard g(m_mutex);
int rc = false; int rc = false;
if (pHandler != NULL) if (pHandler != NULL)
{ {
...@@ -236,12 +250,14 @@ Logger::removeHandler(LogHandler* pHandler) ...@@ -236,12 +250,14 @@ Logger::removeHandler(LogHandler* pHandler)
void void
Logger::removeAllHandlers() Logger::removeAllHandlers()
{ {
Guard g(m_mutex);
m_pHandlerList->removeAll(); m_pHandlerList->removeAll();
} }
bool bool
Logger::isEnable(LoggerLevel logLevel) const Logger::isEnable(LoggerLevel logLevel) const
{ {
Guard g(m_mutex);
if (logLevel == LL_ALL) if (logLevel == LL_ALL)
{ {
for (unsigned i = 1; i < MAX_LOG_LEVELS; i++) for (unsigned i = 1; i < MAX_LOG_LEVELS; i++)
...@@ -255,6 +271,7 @@ Logger::isEnable(LoggerLevel logLevel) const ...@@ -255,6 +271,7 @@ Logger::isEnable(LoggerLevel logLevel) const
void void
Logger::enable(LoggerLevel logLevel) Logger::enable(LoggerLevel logLevel)
{ {
Guard g(m_mutex);
if (logLevel == LL_ALL) if (logLevel == LL_ALL)
{ {
for (unsigned i = 0; i < MAX_LOG_LEVELS; i++) for (unsigned i = 0; i < MAX_LOG_LEVELS; i++)
...@@ -271,6 +288,7 @@ Logger::enable(LoggerLevel logLevel) ...@@ -271,6 +288,7 @@ Logger::enable(LoggerLevel logLevel)
void void
Logger::enable(LoggerLevel fromLogLevel, LoggerLevel toLogLevel) Logger::enable(LoggerLevel fromLogLevel, LoggerLevel toLogLevel)
{ {
Guard g(m_mutex);
if (fromLogLevel > toLogLevel) if (fromLogLevel > toLogLevel)
{ {
LoggerLevel tmp = toLogLevel; LoggerLevel tmp = toLogLevel;
...@@ -287,6 +305,7 @@ Logger::enable(LoggerLevel fromLogLevel, LoggerLevel toLogLevel) ...@@ -287,6 +305,7 @@ Logger::enable(LoggerLevel fromLogLevel, LoggerLevel toLogLevel)
void void
Logger::disable(LoggerLevel logLevel) Logger::disable(LoggerLevel logLevel)
{ {
Guard g(m_mutex);
if (logLevel == LL_ALL) if (logLevel == LL_ALL)
{ {
for (unsigned i = 0; i < MAX_LOG_LEVELS; i++) for (unsigned i = 0; i < MAX_LOG_LEVELS; i++)
...@@ -359,6 +378,7 @@ Logger::debug(const char* pMsg, ...) const ...@@ -359,6 +378,7 @@ Logger::debug(const char* pMsg, ...) const
void void
Logger::log(LoggerLevel logLevel, const char* pMsg, va_list ap) const Logger::log(LoggerLevel logLevel, const char* pMsg, va_list ap) const
{ {
Guard g(m_mutex);
if (m_logLevels[LL_ON] && m_logLevels[logLevel]) if (m_logLevels[LL_ON] && m_logLevels[logLevel])
{ {
char buf[MAX_LOG_MESSAGE_SIZE]; char buf[MAX_LOG_MESSAGE_SIZE];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment