Make our log functions thread safe

diff --git a/CMakeLists.txt b/CMakeLists.txt
index b30537b..144c624 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 <unistd.h>
 #include <string.h>
 #include <boost/shared_ptr.hpp>
+#include <boost/thread/recursive_mutex.hpp>
+#include <boost/thread/thread.hpp>
 
 #include <stringfunc.hxx>
 #include <filefunc.hxx>
@@ -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)