From 5792472587302121f101ff31673e8d30f73ebd75 Mon Sep 17 00:00:00 2001 From: Thomas Jarosch Date: Fri, 15 Jul 2011 15:42:24 +0200 Subject: [PATCH] Make our log functions thread safe --- CMakeLists.txt | 2 +- src/CMakeLists.txt | 2 +- src/logfunc.cpp | 73 ++++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 61 insertions(+), 16 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b30537b..144c624 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -114,7 +114,7 @@ endif(IMAP_UTF7_SUPPORT) include(FindPkgConfig) # Find Boost -find_package(Boost COMPONENTS iostreams unit_test_framework REQUIRED) +find_package(Boost COMPONENTS iostreams unit_test_framework thread REQUIRED) # Find libxml++ if (BUILD_XMLLIB) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9738ebb..0e743e8 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -57,7 +57,7 @@ SET(cpp_headers add_library(i2ncommon SHARED ${cpp_sources} ${cpp_headers}) -target_link_libraries(i2ncommon ${Boost_IOSTREAMS_LIBRARIES} ${ICONV_LIBRARIES}) +target_link_libraries(i2ncommon ${Boost_IOSTREAMS_LIBRARIES} ${Boost_THREAD_LIBRARIES} ${ICONV_LIBRARIES}) set_target_properties(i2ncommon PROPERTIES VERSION ${VERSION} SOVERSION 3) diff --git a/src/logfunc.cpp b/src/logfunc.cpp index 0975130..ba9f02f 100644 --- a/src/logfunc.cpp +++ b/src/logfunc.cpp @@ -21,9 +21,9 @@ on this file might be covered by the GNU General Public License. * @brief implementaton of logging functions. * * @copyright © Copyright 2007-2008 by Intra2net AG - * - * @note This module is not thread safe! - * @todo make this module thread safe (only useful when we can use threads...) + * + * @note Don't call loggers from global constructed objects + * as this depends on the global object construction sequence. */ #include "logfunc.hpp" @@ -37,6 +37,8 @@ on this file might be covered by the GNU General Public License. #include #include #include +#include +#include #include #include @@ -47,6 +49,9 @@ namespace I2n namespace Logger { +/// Global lock for the logger. Used during all syslog operations +/// and modification of our shared local variables. +boost::recursive_mutex LoggerLock; /** * @brief the global logger instance. @@ -64,21 +69,20 @@ namespace ** local globals: */ -std::string g_ident; -Facility g_facility; -bool g_syslog_opened = false; -int g_max_level= LogLevel::Warning; - -bool g_stderr_log = false; +static std::string g_ident; +static Facility g_facility; +static bool g_syslog_opened = false; +static int g_max_level= LogLevel::Warning; -std::string g_log_file_name; -boost::shared_ptr< std::ofstream > g_log_stream_ptr; +static bool g_stderr_log = false; +static std::string g_log_file_name; +static boost::shared_ptr< std::ofstream > g_log_stream_ptr; /** * @brief lookup array for translating our log levels to syslog level. */ -int loglevel_2_syslog_level[ LogLevel::_LogLevel_END ] = { +static int loglevel_2_syslog_level[ LogLevel::_LogLevel_END ] = { LOG_EMERG, LOG_ALERT, LOG_CRIT, @@ -95,7 +99,7 @@ int loglevel_2_syslog_level[ LogLevel::_LogLevel_END ] = { * * These tags are used when logs are written to stderr or into a log file. */ -std::string loglevel_2_short_tag[ LogLevel::_LogLevel_END ] = { +static std::string loglevel_2_short_tag[ LogLevel::_LogLevel_END ] = { "EMRG", "ALRT", "CRIT", @@ -113,7 +117,7 @@ std::string loglevel_2_short_tag[ LogLevel::_LogLevel_END ] = { * Keeping a copy of this ident is necessary since openlog doen't copy it's first * argument but copies only the pointer! (what a **censored**!) */ -char* syslog_ident= NULL; +static char* syslog_ident= NULL; /* @@ -125,6 +129,8 @@ char* syslog_ident= NULL; */ void close_syslog() { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + if (g_syslog_opened) { closelog(); @@ -143,6 +149,8 @@ void close_syslog() */ void open_syslog() { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + close_syslog(); syslog_ident= strdup(g_ident.c_str()); openlog( syslog_ident, LOG_CONS|LOG_PID, g_facility); @@ -157,6 +165,8 @@ void open_syslog() */ int get_syslog_level( int level ) { + // Note: Thread safe + if (level >=0 && level < LogLevel::_LogLevel_END) { return loglevel_2_syslog_level[level]; @@ -172,6 +182,8 @@ int get_syslog_level( int level ) */ std::string get_level_tag( int level ) { + // Note: Thread safe + if (level >=0 && level < LogLevel::_LogLevel_END) { return loglevel_2_short_tag[level]; @@ -195,6 +207,8 @@ std::string get_level_tag( int level ) */ void log_msg( int level, const std::string& msg) { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + if (not g_syslog_opened and not g_stderr_log and not g_log_stream_ptr) { // if nothing is opened for logging: we activate syslog! @@ -269,6 +283,8 @@ void log_part_msg( const std::string& part, const std::string& msg) { + // Note: Locking is done in log_msg() + if (!part.empty()) { std::ostringstream ostr; @@ -292,6 +308,8 @@ void log_part_msg( */ std::string get_program_name() { + // Note: Thread safe + std::string result; // determine the program name: { @@ -323,6 +341,7 @@ std::string get_program_name() void _cleanup() { + // Note: Locking is done in close_syslog(); close_syslog(); //TODO other cleanups? } // _cleanup @@ -458,6 +477,8 @@ PartLogger::~PartLogger() */ void PartLogger::log(int level, const std::string msg) { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + if (level <= g_max_level) { log_part_msg(level, Part, msg); @@ -573,6 +594,8 @@ PartLogger::LogHelper PartLogger::debug(const SourceLocation& loc) */ void enable_syslog( const std::string& name, Facility facility ) { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + close_syslog(); g_ident= name; g_facility= facility; @@ -588,6 +611,8 @@ void enable_syslog( const std::string& name, Facility facility ) */ void enable_syslog( Facility facility ) { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + if (g_ident.empty()) { g_ident= get_program_name(); @@ -604,6 +629,8 @@ void enable_syslog( Facility facility ) */ void enable_syslog( bool enable ) { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + if (enable) { if (!g_syslog_opened) @@ -624,6 +651,8 @@ void enable_syslog( bool enable ) */ void enable_stderr_log(bool enable) { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + g_stderr_log= enable; } // eo enableStderr; @@ -637,6 +666,8 @@ void enable_stderr_log(bool enable) */ void enable_log_file( const std::string& name ) { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + g_log_file_name= name; g_log_stream_ptr.reset( new std::ofstream() ); g_log_stream_ptr->open( name.c_str(), std::ios::out|std::ios::app ); @@ -652,6 +683,8 @@ void enable_log_file( const std::string& name ) */ void enable_log_file( bool enable ) { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + if (enable) { if (g_log_file_name.empty()) @@ -678,6 +711,8 @@ void enable_log_file( bool enable ) */ bool is_logging_to_file() { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + return g_log_stream_ptr and g_log_stream_ptr->good(); } // eo is_logging_to_file() @@ -692,6 +727,8 @@ bool is_logging_to_file() */ std::string get_log_file_name() { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + return g_log_file_name; } // eo get_log_file_name() @@ -701,6 +738,8 @@ std::string get_log_file_name() */ void reopen() { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + if (g_log_stream_ptr) { enable_log_file(false); // closes log, but holds the name. @@ -716,6 +755,8 @@ void reopen() */ int set_log_level(int level) { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + int previous = g_max_level; // Sanity check @@ -736,6 +777,8 @@ int set_log_level(int level) */ int get_log_level() { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + return g_max_level; } // eo get_log_level() @@ -748,6 +791,8 @@ int get_log_level() */ bool has_log_level(int level) { + boost::recursive_mutex::scoped_lock lock(LoggerLock); + return (g_max_level >= level); } // eo has_log_level(int) -- 1.7.1