[trac558] Seond and final stage of backing out log4cxx support

Introduced an implementation class that writes its output to stdout.
It does implement a logger hierarchy, but will be slower at logging
if setting a logging configuration for anything other than the
root logger.

configure.ac

@@ -366,46 +366,44 @@ AC_SUBST(USE_LCOV)
 # Configure log4cxx header and library path
 #
 # If explicitly specified, use it.
-
-AC_ARG_WITH([log4cxx],
-  AC_HELP_STRING([--with-log4cxx=PATH],
-    [specify directory where log4cxx is installed]),
-  [
-   log4cxx_include_path="${withval}/include";
-   log4cxx_library_path="${withval}/lib"
-  ])
-
-# If not specified, try some common paths.  These default to
-# /usr/include and /usr/lib if not found
-
-if test -z "$with_log4cxx"; then
-	log4cxxdirs="/usr/local /usr/pkg /opt /opt/local"
-	for d in $log4cxxdirs
-	do
-		if test -d $d/include/log4cxx; then
-			log4cxx_include_path=$d/include
-			log4cxx_library_path=$d/lib
-			break
-		fi
-	done
-fi
-
-CPPFLAGS_SAVES="$CPPFLAGS"
-if test "${log4cxx_include_path}" ; then
-	LOG4CXX_INCLUDES="-I${log4cxx_include_path}"
-	CPPFLAGS="$CPPFLAGS $LOG4CXX_INCLUDES"
-fi
-AC_CHECK_HEADER([log4cxx/logger.h],, AC_MSG_ERROR([Missing log4cxx header files.]))
-CPPFLAGS="$CPPFLAGS_SAVES"
-AC_SUBST(LOG4CXX_INCLUDES)
-
-LOG4CXX_LDFLAGS="-llog4cxx";
-if test "${log4cxx_library_path}"; then
-    LOG4CXX_LDFLAGS="-L${log4cxx_library_path} -llog4cxx"
-fi
-AC_SUBST(LOG4CXX_LDFLAGS)
-
-
+# 
+# AC_ARG_WITH([log4cxx],
+#   AC_HELP_STRING([--with-log4cxx=PATH],
+#     [specify directory where log4cxx is installed]),
+#   [
+#    log4cxx_include_path="${withval}/include";
+#    log4cxx_library_path="${withval}/lib"
+#   ])
+# 
+# # If not specified, try some common paths.  These default to
+# # /usr/include and /usr/lib if not found
+# 
+# if test -z "$with_log4cxx"; then
+# 	log4cxxdirs="/usr/local /usr/pkg /opt /opt/local"
+# 	for d in $log4cxxdirs
+# 	do
+# 		if test -d $d/include/log4cxx; then
+# 			log4cxx_include_path=$d/include
+# 			log4cxx_library_path=$d/lib
+# 			break
+# 		fi
+# 	done
+# fi
+# 
+# CPPFLAGS_SAVES="$CPPFLAGS"
+# if test "${log4cxx_include_path}" ; then
+# 	LOG4CXX_INCLUDES="-I${log4cxx_include_path}"
+# 	CPPFLAGS="$CPPFLAGS $LOG4CXX_INCLUDES"
+# fi
+# AC_CHECK_HEADER([log4cxx/logger.h],, AC_MSG_ERROR([Missing log4cxx header files.]))
+# CPPFLAGS="$CPPFLAGS_SAVES"
+# AC_SUBST(LOG4CXX_INCLUDES)
+# 
+# LOG4CXX_LDFLAGS="-llog4cxx";
+# if test "${log4cxx_library_path}"; then
+#     LOG4CXX_LDFLAGS="-L${log4cxx_library_path} -llog4cxx"
+# fi
+# AC_SUBST(LOG4CXX_LDFLAGS)
 
 #
 # Configure Boost header path
@@ -811,8 +809,8 @@ dnl includes too
                  ${PYTHON_LDFLAGS}
                  ${PYTHON_LIB}
   Boost:         ${BOOST_INCLUDES}
-  log4cxx:       ${LOG4CXX_INCLUDES}
-                 ${LOG4CXX_LDFLAGS}
+dnl  log4cxx:       ${LOG4CXX_INCLUDES}
+dnl                 ${LOG4CXX_LDFLAGS}
   SQLite:        $SQLITE_CFLAGS
                  $SQLITE_LIBS
 

src/lib/log/Makefile.am

@@ -2,7 +2,7 @@ SUBDIRS = . compiler tests
 
 AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES)
-AM_CPPFLAGS += $(LOG4CXX_INCLUDES)
+# AM_CPPFLAGS += $(LOG4CXX_INCLUDES)
 AM_CPPFLAGS += -I$(top_srcdir)/src/lib/log -I$(top_builddir)/src/lib/log
 
 CLEANFILES = *.gcno *.gcda
@@ -23,9 +23,9 @@ liblog_la_SOURCES += message_reader.cc message_reader.h
 liblog_la_SOURCES += message_types.h
 liblog_la_SOURCES += root_logger_name.cc root_logger_name.h
 liblog_la_SOURCES += strutil.h strutil.cc
-liblog_la_SOURCES += xdebuglevel.cc xdebuglevel.h
+# liblog_la_SOURCES += xdebuglevel.cc xdebuglevel.h
 
-liblog_la_LDFLAGS = $(LOG4CXX_LDFLAGS)
+# liblog_la_LDFLAGS = $(LOG4CXX_LDFLAGS)
 
 # Note: the ordering matters: -Wno-... must follow -Wextra (defined in
 # B10_CXXFLAGS)

src/lib/log/documentation.txt

@@ -216,18 +216,8 @@ To use the current version of the logging:
 
        isc::log::Logger logger("myname", true);
 
-   This is due to an apparent bug in the underlying log4cxx, where the deletion
-   of a statically-declared object at program termination can cause a segment
-   fault. (The destruction of internal memory structures can sometimes happen
-   out of order.)  By default the Logger class creates the structures in its
-   constructor but does not delete them in the destruction.  The default
-   behavious works because instead of reclaiming memory at program run-down,
-   the operating system reclaims it when the process is deleted.
-
-   Setting the second argument "true" causes the Logger's destructor to delete
-   the log4cxx structures.  This does not cause a problem if the program is
-   not terminating.  So use the second form when declaring an automatic
-   instance of isc::log::Logger on the stack.
+   The argument is ignored for underlying implementations other than log4cxx.
+   See below for the use of this argument.
 
 3. The main program unit should include a call to isc::log::runTimeInit()
    (defined in logger_support.h) to set the logging severity, debug log level,
@@ -235,12 +225,12 @@ To use the current version of the logging:
 
    a) The logging severity is one of the enum defined in logger.h, i.e.
 
-        isc::log::Logger::DEBUG
-        isc::log::Logger::INFO
-        isc::log::Logger::WARN
-        isc::log::Logger::ERROR
-        isc::log::Logger::FATAL
-        isc::log::Logger::NONE
+        isc::log::DEBUG
+        isc::log::INFO
+        isc::log::WARN
+        isc::log::ERROR
+        isc::log::FATAL
+        isc::log::NONE
 
    b) The debug log level is only interpreted when the severity is DEBUG and
       is an integer raning from 0 to 99.  0 should be used for the highest-level
@@ -348,16 +338,8 @@ DEBUG is specifically chosen.
 b) Record system-related and packet-related messages via different loggers
 (e.g.  in the example given, sever events could be logged using the logger
 "auth" and packet-related events at that level logged using the logger
-"pkt-auth".)
-As the loggers are independent and the severity levels independent, fine-tuning
-of what and what is not recorded can be achieved.
-
-
-Outstanding Issues
-==================
-* Ability to configure system according to configuration database.
-* Update the build procedure to create .cc and .h files from the .msg file
-  during the build process. (Requires that the message compiler is built first.)
+"pkt-auth".)  As the loggers are independent and the severity levels
+independent, fine-tuning of what and what is not recorded can be achieved.
 
 
 Notes
@@ -369,3 +351,67 @@ the server starts up (or when triggered by a command) to read in a message
 file to overwrite the internal dictionary.  Writing it in C++ means there
 is only one piece of code that does this functionality.
 
+
+Outstanding Issues
+==================
+* Ability to configure system according to configuration database.
+* Update the build procedure to create .cc and .h files from the .msg file
+  during the build process. (Requires that the message compiler is built
+  first.)
+
+
+log4cxx Issues
+==============
+
+Second Argument in Logger Constructor
+-------------------------------------
+As noted above, when using log4cxx as the underlying implementation, the
+argument to the logger's constructor should be set true if declaring the
+logger within a method and set false (or omitted) if declaring the logger
+external to an execution unit.
+
+This is due to an apparent bug in the underlying log4cxx, where the deletion
+of a statically-declared object at program termination can cause a segment
+fault. (The destruction of internal memory structures can sometimes happen
+out of order.)  By default the Logger class creates the structures in
+its constructor but does not delete them in the destruction.  The default
+behavious works because instead of reclaiming memory at program run-down,
+the operating system reclaims it when the process is deleted.
+
+Setting the second argument "true" causes the Logger's destructor to delete
+the log4cxx structures.  This does not cause a problem if the program is
+not terminating.  So use the second form when declaring an automatic instance
+of isc::log::Logger on the stack.
+
+Building with log4cxx
+---------------------
+Owing to issues with versions of log4cxx on different systems, log4cxx was
+temporarily disabled.  To use log4cxx on your system:
+
+* Uncomment the log4cxx lines in configure.ac
+* In src/lib/log, replace the logger_impl.{cc,h} files with their log4cxx
+  equivalents, i.e.
+
+  cp logger_impl_log4cxx.h logger_impl.h
+  cp logger_impl_log4cxx.cc logger_impl.cc
+
+* In src/lib/log/Makefile.am, uncomment the lines:
+
+  # AM_CPPFLAGS += $(LOG4CXX_INCLUDES)
+
+  # liblog_la_SOURCES += xdebuglevel.cc xdebuglevel.h
+
+  # liblog_la_LDFLAGS = $(LOG4CXX_LDFLAGS)
+
+* In src/lib/log/test, re-enable testing of the log4cxx implementation
+  class, i.e.
+
+  cp logger_impl_log4cxx_unittest.cc logger_impl_unittest.cc
+
+  ... and uncomment the following lines in Makefile.am:
+
+  # run_unittests_SOURCES += logger_impl_unittest.cc
+
+  # run_unittests_SOURCES += xdebuglevel_unittest.cc
+
+Then rebuild the system from scratch.

src/lib/log/logger.cc

@@ -173,16 +173,16 @@ Logger::fatal(isc::log::MessageID ident, ...) {
     }
 }
 
-// Protected methods (used for testing)
-
 bool Logger::operator==(const Logger& other) {
     return (*loggerptr_ == *other.loggerptr_);
 }
 
-bool Logger::isInitialized() {
-    return loggerptr_->isInitialized();
-}
+// Protected methods (used for testing)
 
+void
+Logger::reset() {
+    LoggerImpl::reset();
+}
 
 } // namespace log
 } // namespace isc

src/lib/log/logger.h

@@ -80,7 +80,7 @@ public:
     /// enables their deletion - which causes no issues as the problem only
     /// manifests itself during program rundown.
     /// \n
-    /// The falg has no effect on non-log4cxx implementations.
+    /// The flag has no effect on non-log4cxx implementations.
     Logger(const std::string& name, bool infunc = false);
 
 
@@ -188,8 +188,6 @@ public:
     /// \param ... Optional arguments for the message.
     void fatal(MessageID ident, ...);
 
-protected:
-
     /// \brief Equality
     ///
     /// Check if two instances of this logger refer to the same stream.
@@ -198,14 +196,13 @@ protected:
     /// \return true if the logger objects are instances of the same logger.
     bool operator==(const Logger& other);
 
-    
-    /// \brief Logger Initialized
-    ///
-    /// Check that the logger has been properly initialized.  (This method
-    /// is principally for testing.)
+protected:
+
+    /// \brief Reset Global Data
     ///
-    /// \return true if this logger object has been initialized.
-    bool isInitialized();
+    /// Used for testing, this calls upon the underlying logger implementation
+    /// to clear any global data.
+    static void reset();
 
 private:
     LoggerImpl*     loggerptr_;     /// Pointer to the underlying logger

src/lib/log/logger_impl.cc

@@ -15,22 +15,19 @@
 // $Id$
 
 #include <iostream>
+#include <algorithm>
 
 #include <stdarg.h>
 #include <stdio.h>
 
-#include <log4cxx/appender.h>
-#include <log4cxx/basicconfigurator.h>
-#include <log4cxx/patternlayout.h>
-#include <log4cxx/consoleappender.h>
-
+#include <log/debug_levels.h>
 #include <log/root_logger_name.h>
 #include <log/logger.h>
 #include <log/logger_impl.h>
 #include <log/message_dictionary.h>
 #include <log/message_types.h>
+#include <log/root_logger_name.h>
 #include <log/strutil.h>
-#include <log/xdebuglevel.h>
 
 using namespace std;
 
@@ -39,205 +36,221 @@ namespace log {
 
 // Static initializations
 
-bool LoggerImpl::init_ = false;
+LoggerImpl::LoggerInfoMap LoggerImpl::logger_info_;
+LoggerImpl::LoggerInfo LoggerImpl::root_logger_info_(isc::log::INFO, 0);
 
-// Destructor.  Delete log4cxx stuff if "don't delete" is clear.
+// Destructor. (Here because of virtual declaration.)
 
 LoggerImpl::~LoggerImpl() {
-    if (exit_delete_) {
-        delete loggerptr_;
+}
+
+// Return the name of the logger (fully-qualified with the root name if
+// possible).
+
+string LoggerImpl::getName() {
+    string root_name = RootLoggerName::getName();
+    if (root_name.empty() || (name_ == root_name)) {
+        return (name_);
+    }
+    else {
+        return (root_name + "." + name_);
     }
 }
 
-// Initialize logger - create a logger as a child of the root logger.  With
-// log4cxx this is assured by naming the logger <parent>.<child>.
+// Set the severity for logging.
 
 void
-LoggerImpl::initLogger() {
-
-    // Initialize basic logging if not already done.  This is a one-off for
-    // all loggers.
-    if (!init_) {
-
-        // TEMPORARY
-        // Add a suitable console logger to the log4cxx root logger.  (This
-        // is the logger at the root of the log4cxx tree, not the BIND-10 root
-        // logger, which is one level down.)  The chosen format is:
-        //
-        // YYYY-MM-DD hh:mm:ss.sss [logger] SEVERITY: text
-        //
-        // As noted, this is a temporary hack: it is done here to ensure that
-        // a suitable output and output pattern is set.  Future versions of the
-        // software will set this based on configuration data.
-
-        log4cxx::LayoutPtr layout(
-            new log4cxx::PatternLayout(
-                "%d{yyyy-MM-DD HH:mm:ss.SSS} %-5p [%c] %m\n"));
-        log4cxx::AppenderPtr console(
-            new log4cxx::ConsoleAppender(layout));
-        log4cxx::LoggerPtr sys_root_logger = log4cxx::Logger::getRootLogger();
-        sys_root_logger->addAppender(console);
+LoggerImpl::setSeverity(isc::log::Severity severity, int dbglevel) {
+
+    // Silently coerce the debug level into the valid range of 0 to 99
+
+    int debug_level = max(MIN_DEBUG_LEVEL, min(MAX_DEBUG_LEVEL, dbglevel));
+    if (isRootLogger()) {
         
-        // Set the default logging to INFO
-        sys_root_logger->setLevel(log4cxx::Level::getInfo());
+        // Can only set severity for the root logger, you can't disable it.
+        // Any attempt to do so is silently ignored.
+        if (severity != isc::log::DEFAULT) {
+            root_logger_info_ = LoggerInfo(severity, debug_level);
+        }
+
+    } else if (severity == isc::log::DEFAULT) {
+
+        // Want to set to default; this means removing the information
+        // about this logger from the logger_info_ if it is set.
+        LoggerInfoMap::iterator i = logger_info_.find(name_);
+        if (i != logger_info_.end()) {
+            logger_info_.erase(i);
+        }
+
+    } else {
 
-        // All static stuff initialized
-        init_ = true;
+        // Want to set this information
+        logger_info_[name_] = LoggerInfo(severity, debug_level);
     }
+}
 
-    // Initialize this logger.  Name this as to whether the BIND-10 root logger
-    // name has been set.  (If not, this mucks up the hierarchy :-( ).
-    string root_name = RootLoggerName::getName();
-    if (root_name.empty() || (name_ == root_name)) {
-        loggerptr_ = new log4cxx::LoggerPtr(log4cxx::Logger::getLogger(name_));
+// Return severity level
+
+isc::log::Severity
+LoggerImpl::getSeverity() {
+
+    if (isRootLogger()) {
+        return (root_logger_info_.severity);
     }
     else {
-        loggerptr_ = new log4cxx::LoggerPtr(
-            log4cxx::Logger::getLogger(root_name + "." + name_)
-        );
+        LoggerInfoMap::iterator i = logger_info_.find(name_);
+        if (i != logger_info_.end()) {
+           return ((i->second).severity);
+        }
+        else {
+            return (isc::log::DEFAULT);
+        }
     }
 }
 
+// Get effective severity.  Either the current severity or, if not set, the
+// severity of the root level.
 
-// Set the severity for logging.  There is a 1:1 mapping between the logging
-// severity and the log4cxx logging levels, apart from DEBUG.
-//
-// In log4cxx, each of the logging levels (DEBUG, INFO, WARN etc.) has a numeric
-// value.  The level is set to one of these and any numeric level equal to or
-// above it that is reported.  For example INFO has a value of 20000 and ERROR
-// a value of 40000. So if a message of WARN severity (= 30000) is logged, it is
-// not logged when the logger's severity level is ERROR (as 30000 !>= 40000).
-// It is reported if the logger's severity level is set to WARN (as 30000 >=
-/// 30000) or INFO (30000 >= 20000).
-//
-// This gives a simple system for handling different debug levels.  The debug
-// level is a number between 0 and 99, with 0 being least verbose and 99 the
-// most.  To implement this seamlessly, when DEBUG is set, the numeric value
-// of the logging level is actually set to (DEBUG - debug-level).  Similarly
-// messages of level "n" are logged at a logging level of (DEBUG - n).  Thus if
-// the logging level is set to DEBUG and the debug level set to 25, the actual
-// level set is 10000 - 25 = 99975.
-//
-// Attempting to log a debug message of level 26 is an attempt to log a message
-// of level 10000 - 26 = 9974.  As 9974 !>= 9975, it is not logged.  A
-// message of level 25 is, because 9975 >= 9975.
-//
-// The extended set of logging levels is implemented by the XDebugLevel class.
+isc::log::Severity
+LoggerImpl::getEffectiveSeverity() {
+    if (! isRootLogger()) {
 
-void
-LoggerImpl::setSeverity(isc::log::Severity severity, int dbglevel) {
-    switch (severity) {
-        case NONE:
-            getLogger()->setLevel(log4cxx::Level::getOff());
-            break;
-
-        case FATAL:
-            getLogger()->setLevel(log4cxx::Level::getFatal());
-            break;
-
-        case ERROR:
-            getLogger()->setLevel(log4cxx::Level::getError());
-            break;
-
-        case WARN:
-            getLogger()->setLevel(log4cxx::Level::getWarn());
-            break;
-
-        case INFO:
-            getLogger()->setLevel(log4cxx::Level::getInfo());
-            break;
-
-        case DEBUG:
-            getLogger()->setLevel(
-                log4cxx::XDebugLevel::getExtendedDebug(dbglevel));
-            break;
-
-        // Will get here for DEFAULT or any other value.  This disables the
-        // logger's own severity and it defaults to the severity of the parent
-        // logger.
-        default:
-            getLogger()->setLevel(0);
+        // Not root logger, look this logger up in the map.
+        LoggerInfoMap::iterator i = logger_info_.find(name_);
+        if (i != logger_info_.end()) {
+
+            // Found, so return the severity.
+            return ((i->second).severity);
+        }
     }
+
+    // Must be the root logger, or this logger is defaulting to the root logger
+    // settings.
+    return (root_logger_info_.severity);
 }
 
-// Convert between numeric log4cxx logging level and BIND-10 logging severity.
+// Get the debug level.  This returns 0 unless the severity is DEBUG.
 
-isc::log::Severity
-LoggerImpl::convertLevel(int value) {
-
-    // The order is optimised.  This is only likely to be called when testing
-    // for writing debug messages, so the check for DEBUG_INT is first.
-    if (value <= log4cxx::Level::DEBUG_INT) {
-        return (DEBUG);
-    } else if (value <= log4cxx::Level::INFO_INT) {
-        return (INFO);
-    } else if (value <= log4cxx::Level::WARN_INT) {
-        return (WARN);
-    } else if (value <= log4cxx::Level::ERROR_INT) {
-        return (ERROR);
-    } else if (value <= log4cxx::Level::FATAL_INT) {
-        return (FATAL);
+int
+LoggerImpl::getDebugLevel() {
+
+    if (! isRootLogger()) {
+
+        // Not root logger, look this logger up in the map.
+        LoggerInfoMap::iterator i = logger_info_.find(name_);
+        if (i != logger_info_.end()) {
+
+            // Found, so return the debug level.
+            if ((i->second).severity == isc::log::DEBUG) {
+                return ((i->second).dbglevel);
+            } else {
+                return (0);
+            }
+        }
+    }
+
+    // Must be the root logger, or this logger is defaulting to the root logger
+    // settings.
+    if (root_logger_info_.severity == isc::log::DEBUG) {
+        return (root_logger_info_.dbglevel);
     } else {
-        return (NONE);
+        return (0);
     }
 }
 
+// The code for isXxxEnabled is quite simple and is in the header.  The only
+// exception is isDebugEnabled() where we have the complication of the debug
+// levels.
 
-// Return the logging severity associated with this logger.
+bool
+LoggerImpl::isDebugEnabled(int dbglevel) {
 
-isc::log::Severity
-LoggerImpl::getSeverityCommon(const log4cxx::LoggerPtr& ptrlogger,
-    bool check_parent) {
+    if (logger_info_.empty()) {
 
-    log4cxx::LevelPtr level = ptrlogger->getLevel();
-    if (level == log4cxx::LevelPtr()) {
+        // Nothing set, return information from the root logger.
+        return ((root_logger_info_.severity <= isc::log::DEBUG) &&
+            (root_logger_info_.dbglevel >= dbglevel));
+    }
 
-        // Null level returned, logging should be that of the parent.
+    // Something is in the general logger map, so we need to look up the
+    // information.  We don't use getSeverity() and getDebugLevel() separately
+    // as each involves a lookup in the map, something that we can optimise
+    // here.
 
-        if (check_parent) {
-            log4cxx::LoggerPtr parent = ptrlogger->getParent();
-            if (parent == log4cxx::LoggerPtr()) {
+    if (! isRootLogger()) {
 
-                // No parent, so reached the end of the chain.  Return INFO
-                // severity.
-                return (INFO);
-            }
-            else {
-                return getSeverityCommon(parent, check_parent);
+        // Not root logger, look this logger up in the map.
+        LoggerInfoMap::iterator i = logger_info_.find(name_);
+        if (i != logger_info_.end()) {
+
+            // Found, so return the debug level.
+            if ((i->second).severity <= isc::log::DEBUG) {
+                return ((i->second).dbglevel >= dbglevel);
+            } else {
+                return (false); // Nothing lower than debug
             }
         }
-        else {
-            return (DEFAULT);
-        }
+    }
+
+    // Must be the root logger, or this logger is defaulting to the root logger
+    // settings.
+    if (root_logger_info_.severity <= isc::log::DEBUG) {
+        return (root_logger_info_.dbglevel > dbglevel);
     } else {
-        return convertLevel(level->toInt());
+        return (false);
     }
 }
 
+// Output a general message
 
-// Get the debug level.  This returns 0 unless the severity is DEBUG.
+void
+LoggerImpl::output(const char* sev_text, const string& message) {
+    
+    // Get the time in a struct tm format, and convert to text
+    time_t t_time;
+    time(&t_time);
+    struct tm* tm_time = localtime(&t_time);
+
+    char chr_time[32];
+    (void) strftime(chr_time, sizeof(chr_time), "%Y-%m-%d %H:%M:%S", tm_time);
+    chr_time[sizeof(chr_time) - 1] = '\0';  // Guarantee a trailing NULL
+
+    // Now output.
+    std::cout << chr_time << " " << sev_text << " [" << getName() << "] " <<
+        message << "\n";
+}
 
-int
-LoggerImpl::getDebugLevel() {
+// Output various levels
 
-    log4cxx::LevelPtr level = getLogger()->getLevel();
-    if (level == log4cxx::LevelPtr()) {
+void
+LoggerImpl::debug(MessageID ident, const char* text) {
+    string message = ident + ", " + text;
+    output("DEBUG", message);
+}
 
-        // Null pointer returned, logging should be that of the parent.
-        return (0);
-        
-    } else {
-        int severity = level->toInt();
-        if (severity <= log4cxx::Level::DEBUG_INT) {
-            return (log4cxx::Level::DEBUG_INT - severity);
-        }
-        else {
-            return (0);
-        }
-    }
+void
+LoggerImpl::info(MessageID ident, const char* text) {
+    string message = ident + ", " + text;
+    output("INFO ", message);
 }
 
+void
+LoggerImpl::warn(MessageID ident, const char* text) {
+    string message = ident + ", " + text;
+    output("WARN ", message);
+}
 
+void
+LoggerImpl::error(MessageID ident, const char* text) {
+    string message = ident + ", " + text;
+    output("ERROR", message);
+}
+
+void
+LoggerImpl::fatal(MessageID ident, const char* text) {
+    string message = ident + ", " + text;
+    output("FATAL", message);
+}
 
 } // namespace log
 } // namespace isc

src/lib/log/logger_impl.h

@@ -17,66 +17,66 @@
 #ifndef __LOGGER_IMPL_H
 #define __LOGGER_IMPL_H
 
+#include <time.h>
+
 #include <cstdlib>
+#include <iostream>
 #include <string>
-#include <boost/lexical_cast.hpp>
-#include <log4cxx/logger.h>
-#include <log4cxx/logger.h>
+#include <map>
+#include <utility>
 
 #include <log/debug_levels.h>
 #include <log/logger.h>
 #include <log/message_types.h>
+#include <log/root_logger_name.h>
 
 namespace isc {
 namespace log {
 
-/// \brief Log4cxx Logger Implementation
+/// \brief Console Logger Implementation
 ///
 /// The logger uses a "pimpl" idiom for implementation, where the base logger
 /// class contains little more than a pointer to the implementation class, and
 /// all actions are carried out by the latter.  This class is an implementation
-/// class interfacing to the log4cxx logging system.
+/// class that just outputs to stdout.
 
 class LoggerImpl {
 public:
 
+    /// \brief Information About Logger
+    ///
+    /// Holds a information about a logger, namely its severity and its debug
+    /// level.  This could be a std::pair, except that it gets confusing when
+    /// accessing the LoggerInfoMap: that returns a pair, so we to reference
+    /// elements we would use constructs like ((i->first).second);
+    struct LoggerInfo {
+        isc::log::Severity  severity;
+        int                 dbglevel;
+
+        LoggerInfo(isc::log::Severity sev = isc::log::INFO,
+            int dbg = MIN_DEBUG_LEVEL) : severity(sev), dbglevel(dbg)
+        {}
+    };
+
+
+    /// \brief Information About All Loggers
+    ///
+    /// Information about all loggers in the system - except the root logger -
+    /// is held in a map, linking name of the logger (excluding the root
+    /// name component) and its set severity and debug levels.  The root
+    /// logger information is held separately.
+    typedef std::map<std::string, LoggerInfo>   LoggerInfoMap;
+
+
     /// \brief Constructor
     ///
-    /// Creates/attaches to a logger of a specific name.
+    /// Creates a logger of the specific name.
     ///
-    /// \param name Name of the logger.  If the name is that of the root name,
-    /// this creates an instance of the root logger; otherwise it creates a
-    /// child of the root logger.
+    /// \param name Name of the logger.
     ///
     /// \param exit_delete This argument is present to get round a bug in
-    /// log4cxx.  If a log4cxx logger is declared outside an execution unit, it
-    /// is not deleted until the program runs down.  At that point all such
-    /// objects - including internal log4cxx objects - are deleted.  However,
-    /// there seems to be a bug in log4cxx where the way that such objects are
-    /// destroyed causes a MutexException to be thrown (this is described in
-    /// https://issues.apache.org/jira/browse/LOGCXX-322).  As this only occurs
-    /// during program rundown, the issue is not serious - it just looks bad to
-    /// have the program crash instead of shut down cleanly.\n
-    /// \n
-    /// The original implementation of the isc::log::Logger had as a member a
-    /// log4cxx logger (actually a LoggerPtr).  If the isc:: Logger was declared
-    /// statically, when it was destroyed at the end of the program the internal
-    /// LoggerPtr was destroyed, which triggered the problem.  The problem did
-    /// not occur if the isc::log::Logger was created on the stack.  To get
-    /// round this, the internal LoggerPtr is now created dynamically.  The
-    /// exit_delete argument controls its destruction: if true, it is destroyed
-    /// in the ISC Logger destructor.  If false, it is not.\n
-    /// \n
-    /// When creating an isc::log::Logger on the stack, the argument should be
-    /// false (the default); when the Logger is destroyed, all the internal
-    /// log4cxx objects are destroyed.  As only the logger (and not the internal
-    /// log4cxx data structures are being destroyed), all is well.  However,
-    /// when creating the logger statically, the argument should be false.  This
-    /// means that the log4cxx objects are not destroyed at program rundown;
-    /// instead memory is reclaimed and files are closed when the process is
-    /// destroyed, something that does not trigger the bug.
-    LoggerImpl(const std::string& name, bool exit_delete = false) :
-        loggerptr_(NULL), name_(name), exit_delete_(exit_delete)
+    /// the log4cxx implementation.  It is unused here.
+    LoggerImpl(const std::string& name, bool) : name_(name)
     {}
 
 
@@ -85,8 +85,15 @@ public:
 
 
     /// \brief Get the full name of the logger (including the root name)
-    virtual std::string getName() {
-        return (getLogger()->getName());
+    virtual std::string getName();
+
+
+    /// \brief Check if this is the root logger
+    ///
+    /// \return true if the name of this logger is the same as that of the
+    /// root logger.
+    virtual bool isRootLogger() const {
+        return (name_ == RootLoggerName::getName());
     }
 
 
@@ -106,9 +113,7 @@ public:
     ///
     /// \return The current logging level of this logger.  In most cases though,
     /// the effective logging level is what is required.
-    virtual isc::log::Severity getSeverity() {
-        return getSeverityCommon(getLogger(), false);
-    }
+    virtual isc::log::Severity getSeverity();
 
 
     /// \brief Get Effective Severity Level for Logger
@@ -116,9 +121,7 @@ public:
     /// \return The effective severity level of the logger.  This is the same
     /// as getSeverity() if the logger has a severity level set, but otherwise
     /// is the severity of the parent.
-    virtual isc::log::Severity getEffectiveSeverity() {
-        return getSeverityCommon(getLogger(), true);
-    }
+    virtual isc::log::Severity getEffectiveSeverity();
 
 
     /// \brief Return DEBUG Level
@@ -134,85 +137,91 @@ public:
     /// enabled only if the logger has DEBUG enabled and if the dbglevel
     /// checked is less than or equal to the debug level set for the logger.
     virtual bool
-    isDebugEnabled(int dbglevel = MIN_DEBUG_LEVEL) {
-        return (getLogger()->getEffectiveLevel()->toInt() <=
-            (log4cxx::Level::DEBUG_INT - dbglevel));
-    }
-
+    isDebugEnabled(int dbglevel = MIN_DEBUG_LEVEL);
 
     /// \brief Is INFO Enabled?
     virtual bool isInfoEnabled() {
-        return (getLogger()->isInfoEnabled());
+        return (isEnabled(isc::log::INFO));
     }
 
-
     /// \brief Is WARNING Enabled?
     virtual bool isWarnEnabled() {
-        return (getLogger()->isWarnEnabled());
+        return (isEnabled(isc::log::WARN));
     }
 
-
     /// \brief Is ERROR Enabled?
     virtual bool isErrorEnabled() {
-        return (getLogger()->isErrorEnabled());
+        return (isEnabled(isc::log::ERROR));
     }
 
-
     /// \brief Is FATAL Enabled?
     virtual bool isFatalEnabled() {
-        return (getLogger()->isFatalEnabled());
+        return (isEnabled(isc::log::FATAL));
+    }
+
+
+    /// \brief Common Severity check
+    ///
+    /// Implements the common severity check.  As an optimisation, this checks
+    /// to see if any logger-specific levels have been set (a quick check as it
+    /// just involves seeing if the collection of logger information is empty).
+    /// if not, it returns the information for the root level; if so, it has
+    /// to take longer and look up the information in the map holding the
+    /// logging details.
+    virtual bool isEnabled(isc::log::Severity severity) {
+        if (logger_info_.empty()) {
+            return (root_logger_info_.severity <= severity);
+        }
+        else {
+            return (getSeverity() <= severity);
+        }
     }
 
 
+    /// \brief Output General Message
+    ///
+    /// The message is formatted to include the date and time, the severity
+    /// and the logger generating the message.
+    ///
+    /// \param sev_text Severity lovel as a text string
+    /// \param text Text to log
+    void output(const char* sev_text, const std::string& message);
+
+
     /// \brief Output Debug Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void debug(MessageID ident, const char* text) {
-        LOG4CXX_DEBUG(getLogger(), ident << ", " << text);
-    }
+    void debug(MessageID ident, const char* text);
 
 
     /// \brief Output Informational Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void info(MessageID ident, const char* text) {
-        LOG4CXX_INFO(getLogger(), ident << ", " << text);
-    }
+    void info(MessageID ident, const char* text);
 
 
     /// \brief Output Warning Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void warn(MessageID ident, const char* text) {
-        LOG4CXX_WARN(getLogger(), ident << ", " << text);
-    }
+    void warn(MessageID ident, const char* text);
 
 
     /// \brief Output Error Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void error(MessageID ident, const char* text) {
-        LOG4CXX_ERROR(getLogger(), ident << ", " << text);
-    }
+    void error(MessageID ident, const char* text);
 
 
     /// \brief Output Fatal Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void fatal(MessageID ident, const char* text) {
-        LOG4CXX_FATAL(getLogger(), ident << ", " << text);
-    }
+    void fatal(MessageID ident, const char* text);
 
-    //@{
-    /// \brief Testing Methods
-    ///
-    /// The next set of methods are used in testing.  As they are accessed from
-    /// the main logger class, they must be public.
 
     /// \brief Equality
     ///
@@ -221,86 +230,30 @@ public:
     ///
     /// \return true if the logger objects are instances of the same logger.
     bool operator==(const LoggerImpl& other) {
-        return (*loggerptr_ == *other.loggerptr_);
+        return (name_ == other.name_);
     }
 
 
-    /// \brief Logger Initialized
+    /// \brief Reset Global Data
     ///
-    /// Check that the logger has been properly initialized.  (This method
-    /// is principally for testing.)
-    ///
-    /// \return true if this logger object has been initialized.
-    bool isInitialized() {
-        return (loggerptr_ != NULL);
+    /// Only used for testing, this clears all the logger information and
+    /// resets it back to default values.
+    static void reset() {
+        root_logger_info_ = LoggerInfo(isc::log::INFO, MIN_DEBUG_LEVEL);
+        logger_info_.clear();
     }
 
-    //@}
-
-protected:
-
-    /// \brief Convert Between BIND-10 and log4cxx Logging Levels
-    ///
-    /// This method is marked protected to allow for unit testing.
-    ///
-    /// \param value log4cxx numeric logging level
-    ///
-    /// \return BIND-10 logging severity
-    isc::log::Severity convertLevel(int value);
 
 private:
-
-    /// \brief Get Severity Level for Logger
-    ///
-    /// This is common code for getSeverity() and getEffectiveSeverity() -
-    /// it returns the severity of the logger; if not set (and the check_parent)
-    /// flag is set, it searches up the parent-child tree until a severity
-    /// level is found and uses that.
-    ///
-    /// \param ptrlogger Pointer to the log4cxx logger to check.
-    /// \param check_parent true to search up the tree, false to return the
-    /// current level.
-    ///
-    /// \return The effective severity level of the logger.  This is the same
-    /// as getSeverity() if the logger has a severity level set, but otherwise
-    /// is the severity of the parent.
-    isc::log::Severity getSeverityCommon(const log4cxx::LoggerPtr& ptrlogger,
-        bool check_parent);
-
-
-
-    /// \brief Initialize log4cxx Logger
-    ///
-    /// Creates the log4cxx logger used internally.  A function is provided for
-    /// this so that the creation does not take place when this Logger object
-    /// is created but when it is used.  As the latter occurs in executable
-    /// code but the former can occur during initialization, this order
-    /// guarantees that anything that is statically initialized has completed
-    /// its initialization by the time the logger is used.
-    void initLogger();
-
-
-    /// \brief Return underlying log4cxx logger, initializing it if necessary
-    ///
-    /// \return Loggerptr object
-    log4cxx::LoggerPtr& getLogger() {
-        if (loggerptr_ == NULL) {
-            initLogger();
-        }
-        return *loggerptr_;
-    }
-
-    // Members.  Note that loggerptr_ is a pointer to a LoggerPtr, which is
-    // itself a pointer to the underlying log4cxx logger.  This is due to the
-    // problems with memory deletion on program exit, explained in the comments
-    // for the "exit_delete" parameter in this class's constructor.
-
-    mutable log4cxx::LoggerPtr*  loggerptr_; ///< Pointer to the underlying logger
-    std::string          name_;         ///< Name of this logger]
-    bool                 exit_delete_;  ///< Delete loggerptr_ on exit?
-
-    // NOTE - THIS IS A PLACE HOLDER
-    static bool         init_;      ///< Set true when initialized
+    std::string          name_;                 ///< Name of this logger
+    
+    // Split the status of the root logger from this logger.  If - is will
+    // probably be the usual case - no per-logger setting is enabled, a
+    // quick check of logger_info_.empty() will return true and we can quickly
+    // return the root logger status without a length lookup in the map.
+
+    static LoggerInfo       root_logger_info_;  ///< Status of root logger
+    static LoggerInfoMap    logger_info_;       ///< Store of debug levels etc.
 };
 
 } // namespace log

src/lib/log/logger_impl_log4cxx.cc

@@ -0,0 +1,243 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE
+
+// $Id$
+
+#include <iostream>
+
+#include <stdarg.h>
+#include <stdio.h>
+
+#include <log4cxx/appender.h>
+#include <log4cxx/basicconfigurator.h>
+#include <log4cxx/patternlayout.h>
+#include <log4cxx/consoleappender.h>
+
+#include <log/root_logger_name.h>
+#include <log/logger.h>
+#include <log/logger_impl.h>
+#include <log/message_dictionary.h>
+#include <log/message_types.h>
+#include <log/strutil.h>
+#include <log/xdebuglevel.h>
+
+using namespace std;
+
+namespace isc {
+namespace log {
+
+// Static initializations
+
+bool LoggerImpl::init_ = false;
+
+// Destructor.  Delete log4cxx stuff if "don't delete" is clear.
+
+LoggerImpl::~LoggerImpl() {
+    if (exit_delete_) {
+        delete loggerptr_;
+    }
+}
+
+// Initialize logger - create a logger as a child of the root logger.  With
+// log4cxx this is assured by naming the logger <parent>.<child>.
+
+void
+LoggerImpl::initLogger() {
+
+    // Initialize basic logging if not already done.  This is a one-off for
+    // all loggers.
+    if (!init_) {
+
+        // TEMPORARY
+        // Add a suitable console logger to the log4cxx root logger.  (This
+        // is the logger at the root of the log4cxx tree, not the BIND-10 root
+        // logger, which is one level down.)  The chosen format is:
+        //
+        // YYYY-MM-DD hh:mm:ss.sss [logger] SEVERITY: text
+        //
+        // As noted, this is a temporary hack: it is done here to ensure that
+        // a suitable output and output pattern is set.  Future versions of the
+        // software will set this based on configuration data.
+
+        log4cxx::LayoutPtr layout(
+            new log4cxx::PatternLayout(
+                "%d{yyyy-MM-DD HH:mm:ss.SSS} %-5p [%c] %m\n"));
+        log4cxx::AppenderPtr console(
+            new log4cxx::ConsoleAppender(layout));
+        log4cxx::LoggerPtr sys_root_logger = log4cxx::Logger::getRootLogger();
+        sys_root_logger->addAppender(console);
+        
+        // Set the default logging to INFO
+        sys_root_logger->setLevel(log4cxx::Level::getInfo());
+
+        // All static stuff initialized
+        init_ = true;
+    }
+
+    // Initialize this logger.  Name this as to whether the BIND-10 root logger
+    // name has been set.  (If not, this mucks up the hierarchy :-( ).
+    string root_name = RootLoggerName::getName();
+    if (root_name.empty() || (name_ == root_name)) {
+        loggerptr_ = new log4cxx::LoggerPtr(log4cxx::Logger::getLogger(name_));
+    }
+    else {
+        loggerptr_ = new log4cxx::LoggerPtr(
+            log4cxx::Logger::getLogger(root_name + "." + name_)
+        );
+    }
+}
+
+
+// Set the severity for logging.  There is a 1:1 mapping between the logging
+// severity and the log4cxx logging levels, apart from DEBUG.
+//
+// In log4cxx, each of the logging levels (DEBUG, INFO, WARN etc.) has a numeric
+// value.  The level is set to one of these and any numeric level equal to or
+// above it that is reported.  For example INFO has a value of 20000 and ERROR
+// a value of 40000. So if a message of WARN severity (= 30000) is logged, it is
+// not logged when the logger's severity level is ERROR (as 30000 !>= 40000).
+// It is reported if the logger's severity level is set to WARN (as 30000 >=
+/// 30000) or INFO (30000 >= 20000).
+//
+// This gives a simple system for handling different debug levels.  The debug
+// level is a number between 0 and 99, with 0 being least verbose and 99 the
+// most.  To implement this seamlessly, when DEBUG is set, the numeric value
+// of the logging level is actually set to (DEBUG - debug-level).  Similarly
+// messages of level "n" are logged at a logging level of (DEBUG - n).  Thus if
+// the logging level is set to DEBUG and the debug level set to 25, the actual
+// level set is 10000 - 25 = 99975.
+//
+// Attempting to log a debug message of level 26 is an attempt to log a message
+// of level 10000 - 26 = 9974.  As 9974 !>= 9975, it is not logged.  A
+// message of level 25 is, because 9975 >= 9975.
+//
+// The extended set of logging levels is implemented by the XDebugLevel class.
+
+void
+LoggerImpl::setSeverity(isc::log::Severity severity, int dbglevel) {
+    switch (severity) {
+        case NONE:
+            getLogger()->setLevel(log4cxx::Level::getOff());
+            break;
+
+        case FATAL:
+            getLogger()->setLevel(log4cxx::Level::getFatal());
+            break;
+
+        case ERROR:
+            getLogger()->setLevel(log4cxx::Level::getError());
+            break;
+
+        case WARN:
+            getLogger()->setLevel(log4cxx::Level::getWarn());
+            break;
+
+        case INFO:
+            getLogger()->setLevel(log4cxx::Level::getInfo());
+            break;
+
+        case DEBUG:
+            getLogger()->setLevel(
+                log4cxx::XDebugLevel::getExtendedDebug(dbglevel));
+            break;
+
+        // Will get here for DEFAULT or any other value.  This disables the
+        // logger's own severity and it defaults to the severity of the parent
+        // logger.
+        default:
+            getLogger()->setLevel(0);
+    }
+}
+
+// Convert between numeric log4cxx logging level and BIND-10 logging severity.
+
+isc::log::Severity
+LoggerImpl::convertLevel(int value) {
+
+    // The order is optimised.  This is only likely to be called when testing
+    // for writing debug messages, so the check for DEBUG_INT is first.
+    if (value <= log4cxx::Level::DEBUG_INT) {
+        return (DEBUG);
+    } else if (value <= log4cxx::Level::INFO_INT) {
+        return (INFO);
+    } else if (value <= log4cxx::Level::WARN_INT) {
+        return (WARN);
+    } else if (value <= log4cxx::Level::ERROR_INT) {
+        return (ERROR);
+    } else if (value <= log4cxx::Level::FATAL_INT) {
+        return (FATAL);
+    } else {
+        return (NONE);
+    }
+}
+
+
+// Return the logging severity associated with this logger.
+
+isc::log::Severity
+LoggerImpl::getSeverityCommon(const log4cxx::LoggerPtr& ptrlogger,
+    bool check_parent) {
+
+    log4cxx::LevelPtr level = ptrlogger->getLevel();
+    if (level == log4cxx::LevelPtr()) {
+
+        // Null level returned, logging should be that of the parent.
+
+        if (check_parent) {
+            log4cxx::LoggerPtr parent = ptrlogger->getParent();
+            if (parent == log4cxx::LoggerPtr()) {
+
+                // No parent, so reached the end of the chain.  Return INFO
+                // severity.
+                return (INFO);
+            }
+            else {
+                return getSeverityCommon(parent, check_parent);
+            }
+        }
+        else {
+            return (DEFAULT);
+        }
+    } else {
+        return convertLevel(level->toInt());
+    }
+}
+
+
+// Get the debug level.  This returns 0 unless the severity is DEBUG.
+
+int
+LoggerImpl::getDebugLevel() {
+
+    log4cxx::LevelPtr level = getLogger()->getLevel();
+    if (level == log4cxx::LevelPtr()) {
+
+        // Null pointer returned, logging should be that of the parent.
+        return (0);
+        
+    } else {
+        int severity = level->toInt();
+        if (severity <= log4cxx::Level::DEBUG_INT) {
+            return (log4cxx::Level::DEBUG_INT - severity);
+        }
+        else {
+            return (0);
+        }
+    }
+}
+
+
+
+} // namespace log
+} // namespace isc

src/lib/log/logger_impl_log4cxx.h

@@ -0,0 +1,317 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+#ifndef __LOGGER_IMPL_H
+#define __LOGGER_IMPL_H
+
+#include <cstdlib>
+#include <string>
+#include <boost/lexical_cast.hpp>
+#include <log4cxx/logger.h>
+#include <log4cxx/logger.h>
+
+#include <log/debug_levels.h>
+#include <log/logger.h>
+#include <log/message_types.h>
+
+namespace isc {
+namespace log {
+
+/// \brief Log4cxx Logger Implementation
+///
+/// The logger uses a "pimpl" idiom for implementation, where the base logger
+/// class contains little more than a pointer to the implementation class, and
+/// all actions are carried out by the latter.  This class is an implementation
+/// class interfacing to the log4cxx logging system.
+
+class LoggerImpl {
+public:
+
+    /// \brief Constructor
+    ///
+    /// Creates/attaches to a logger of a specific name.
+    ///
+    /// \param name Name of the logger.  If the name is that of the root name,
+    /// this creates an instance of the root logger; otherwise it creates a
+    /// child of the root logger.
+    ///
+    /// \param exit_delete This argument is present to get round a bug in
+    /// log4cxx.  If a log4cxx logger is declared outside an execution unit, it
+    /// is not deleted until the program runs down.  At that point all such
+    /// objects - including internal log4cxx objects - are deleted.  However,
+    /// there seems to be a bug in log4cxx where the way that such objects are
+    /// destroyed causes a MutexException to be thrown (this is described in
+    /// https://issues.apache.org/jira/browse/LOGCXX-322).  As this only occurs
+    /// during program rundown, the issue is not serious - it just looks bad to
+    /// have the program crash instead of shut down cleanly.\n
+    /// \n
+    /// The original implementation of the isc::log::Logger had as a member a
+    /// log4cxx logger (actually a LoggerPtr).  If the isc:: Logger was declared
+    /// statically, when it was destroyed at the end of the program the internal
+    /// LoggerPtr was destroyed, which triggered the problem.  The problem did
+    /// not occur if the isc::log::Logger was created on the stack.  To get
+    /// round this, the internal LoggerPtr is now created dynamically.  The
+    /// exit_delete argument controls its destruction: if true, it is destroyed
+    /// in the ISC Logger destructor.  If false, it is not.\n
+    /// \n
+    /// When creating an isc::log::Logger on the stack, the argument should be
+    /// false (the default); when the Logger is destroyed, all the internal
+    /// log4cxx objects are destroyed.  As only the logger (and not the internal
+    /// log4cxx data structures are being destroyed), all is well.  However,
+    /// when creating the logger statically, the argument should be false.  This
+    /// means that the log4cxx objects are not destroyed at program rundown;
+    /// instead memory is reclaimed and files are closed when the process is
+    /// destroyed, something that does not trigger the bug.
+    LoggerImpl(const std::string& name, bool exit_delete = false) :
+        loggerptr_(NULL), name_(name), exit_delete_(exit_delete)
+    {}
+
+
+    /// \brief Destructor
+    virtual ~LoggerImpl();
+
+
+    /// \brief Get the full name of the logger (including the root name)
+    virtual std::string getName() {
+        return (getLogger()->getName());
+    }
+
+
+    /// \brief Set Severity Level for Logger
+    ///
+    /// Sets the level at which this logger will log messages.  If none is set,
+    /// the level is inherited from the parent.
+    ///
+    /// \param severity Severity level to log
+    /// \param dbglevel If the severity is DEBUG, this is the debug level.
+    /// This can be in the range 1 to 100 and controls the verbosity.  A value
+    /// outside these limits is silently coerced to the nearest boundary.
+    virtual void setSeverity(isc::log::Severity severity, int dbglevel = 1);
+
+
+    /// \brief Get Severity Level for Logger
+    ///
+    /// \return The current logging level of this logger.  In most cases though,
+    /// the effective logging level is what is required.
+    virtual isc::log::Severity getSeverity() {
+        return getSeverityCommon(getLogger(), false);
+    }
+
+
+    /// \brief Get Effective Severity Level for Logger
+    ///
+    /// \return The effective severity level of the logger.  This is the same
+    /// as getSeverity() if the logger has a severity level set, but otherwise
+    /// is the severity of the parent.
+    virtual isc::log::Severity getEffectiveSeverity() {
+        return getSeverityCommon(getLogger(), true);
+    }
+
+
+    /// \brief Return DEBUG Level
+    ///
+    /// \return Current setting of debug level.  This is returned regardless of
+    /// whether the
+    virtual int getDebugLevel();
+
+
+    /// \brief Returns if Debug Message Should Be Output
+    ///
+    /// \param dbglevel Level for which debugging is checked.  Debugging is
+    /// enabled only if the logger has DEBUG enabled and if the dbglevel
+    /// checked is less than or equal to the debug level set for the logger.
+    virtual bool
+    isDebugEnabled(int dbglevel = MIN_DEBUG_LEVEL) {
+        return (getLogger()->getEffectiveLevel()->toInt() <=
+            (log4cxx::Level::DEBUG_INT - dbglevel));
+    }
+
+
+    /// \brief Is INFO Enabled?
+    virtual bool isInfoEnabled() {
+        return (getLogger()->isInfoEnabled());
+    }
+
+
+    /// \brief Is WARNING Enabled?
+    virtual bool isWarnEnabled() {
+        return (getLogger()->isWarnEnabled());
+    }
+
+
+    /// \brief Is ERROR Enabled?
+    virtual bool isErrorEnabled() {
+        return (getLogger()->isErrorEnabled());
+    }
+
+
+    /// \brief Is FATAL Enabled?
+    virtual bool isFatalEnabled() {
+        return (getLogger()->isFatalEnabled());
+    }
+
+
+    /// \brief Output Debug Message
+    ///
+    /// \param ident Message identification.
+    /// \param text Text to log
+    void debug(MessageID ident, const char* text) {
+        LOG4CXX_DEBUG(getLogger(), ident << ", " << text);
+    }
+
+
+    /// \brief Output Informational Message
+    ///
+    /// \param ident Message identification.
+    /// \param text Text to log
+    void info(MessageID ident, const char* text) {
+        LOG4CXX_INFO(getLogger(), ident << ", " << text);
+    }
+
+
+    /// \brief Output Warning Message
+    ///
+    /// \param ident Message identification.
+    /// \param text Text to log
+    void warn(MessageID ident, const char* text) {
+        LOG4CXX_WARN(getLogger(), ident << ", " << text);
+    }
+
+
+    /// \brief Output Error Message
+    ///
+    /// \param ident Message identification.
+    /// \param text Text to log
+    void error(MessageID ident, const char* text) {
+        LOG4CXX_ERROR(getLogger(), ident << ", " << text);
+    }
+
+
+    /// \brief Output Fatal Message
+    ///
+    /// \param ident Message identification.
+    /// \param text Text to log
+    void fatal(MessageID ident, const char* text) {
+        LOG4CXX_FATAL(getLogger(), ident << ", " << text);
+    }
+
+    //@{
+    /// \brief Testing Methods
+    ///
+    /// The next set of methods are used in testing.  As they are accessed from
+    /// the main logger class, they must be public.
+
+    /// \brief Equality
+    ///
+    /// Check if two instances of this logger refer to the same stream.
+    /// (This method is principally for testing.)
+    ///
+    /// \return true if the logger objects are instances of the same logger.
+    bool operator==(const LoggerImpl& other) {
+        return (*loggerptr_ == *other.loggerptr_);
+    }
+
+
+    /// \brief Logger Initialized
+    ///
+    /// Check that the logger has been properly initialized.  (This method
+    /// is principally for testing.)
+    ///
+    /// \return true if this logger object has been initialized.
+    bool isInitialized() {
+        return (loggerptr_ != NULL);
+    }
+
+    /// \brief Reset Global Data
+    ///
+    /// Only used for testing, this clears all the logger information and
+    /// resets it back to default values.  This is a no-op for log4cxx.
+    static void reset() {
+    }
+
+    //@}
+
+protected:
+
+    /// \brief Convert Between BIND-10 and log4cxx Logging Levels
+    ///
+    /// This method is marked protected to allow for unit testing.
+    ///
+    /// \param value log4cxx numeric logging level
+    ///
+    /// \return BIND-10 logging severity
+    isc::log::Severity convertLevel(int value);
+
+private:
+
+    /// \brief Get Severity Level for Logger
+    ///
+    /// This is common code for getSeverity() and getEffectiveSeverity() -
+    /// it returns the severity of the logger; if not set (and the check_parent)
+    /// flag is set, it searches up the parent-child tree until a severity
+    /// level is found and uses that.
+    ///
+    /// \param ptrlogger Pointer to the log4cxx logger to check.
+    /// \param check_parent true to search up the tree, false to return the
+    /// current level.
+    ///
+    /// \return The effective severity level of the logger.  This is the same
+    /// as getSeverity() if the logger has a severity level set, but otherwise
+    /// is the severity of the parent.
+    isc::log::Severity getSeverityCommon(const log4cxx::LoggerPtr& ptrlogger,
+        bool check_parent);
+
+
+
+    /// \brief Initialize log4cxx Logger
+    ///
+    /// Creates the log4cxx logger used internally.  A function is provided for
+    /// this so that the creation does not take place when this Logger object
+    /// is created but when it is used.  As the latter occurs in executable
+    /// code but the former can occur during initialization, this order
+    /// guarantees that anything that is statically initialized has completed
+    /// its initialization by the time the logger is used.
+    void initLogger();
+
+
+    /// \brief Return underlying log4cxx logger, initializing it if necessary
+    ///
+    /// \return Loggerptr object
+    log4cxx::LoggerPtr& getLogger() {
+        if (loggerptr_ == NULL) {
+            initLogger();
+        }
+        return *loggerptr_;
+    }
+
+    // Members.  Note that loggerptr_ is a pointer to a LoggerPtr, which is
+    // itself a pointer to the underlying log4cxx logger.  This is due to the
+    // problems with memory deletion on program exit, explained in the comments
+    // for the "exit_delete" parameter in this class's constructor.
+
+    mutable log4cxx::LoggerPtr*  loggerptr_; ///< Pointer to the underlying logger
+    std::string          name_;         ///< Name of this logger]
+    bool                 exit_delete_;  ///< Delete loggerptr_ on exit?
+
+    // NOTE - THIS IS A PLACE HOLDER
+    static bool         init_;      ///< Set true when initialized
+};
+
+} // namespace log
+} // namespace isc
+
+
+#endif // __LOGGER_IMPL_H

src/lib/log/logger_levels.h

@@ -24,6 +24,9 @@ namespace log {
 ///
 /// Defines the severity levels for logging.  This is shared between the logger
 /// and the implementations classes.
+///
+/// N.B. The order of the levels - DEBUG less than INFO less that WARN etc. is
+/// implicitly assumed in several implementations.  They must not be changed.
 
 typedef enum {
     DEFAULT = 0,    // Default to logging level of the parent

src/lib/log/tests/Makefile.am

@@ -17,19 +17,18 @@ TESTS += run_unittests
 run_unittests_SOURCES  = root_logger_name_unittest.cc
 run_unittests_SOURCES += filename_unittest.cc
 run_unittests_SOURCES += logger_unittest.cc
-run_unittests_SOURCES += logger_impl_unittest.cc
+# run_unittests_SOURCES += logger_impl_unittest.cc
 run_unittests_SOURCES += message_dictionary_unittest.cc
 run_unittests_SOURCES += message_reader_unittest.cc
 run_unittests_SOURCES += message_initializer_unittest.cc
 run_unittests_SOURCES += message_initializer_unittest_2.cc
 run_unittests_SOURCES += strutil_unittest.cc
-run_unittests_SOURCES += xdebuglevel_unittest.cc
+# run_unittests_SOURCES += xdebuglevel_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 run_unittests_LDADD  = $(GTEST_LDADD)
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
-run_unittests_LDADD += -llog4cxx
 endif
 
 TESTS += logger_support_test

src/lib/log/tests/logger_unittest.cc

@@ -41,14 +41,8 @@ public:
     TestLogger(const string& name) : Logger(name, true)
     {}
 
-    /// \brief Logger Equality
-    bool operator==(const TestLogger& other) {
-        return Logger::operator==(other);
-    }
-
-    /// \brief Logger is Null
-    bool isInitialized() {
-        return Logger::isInitialized();
+    static void reset() {
+        Logger::reset();
     }
 };
 
@@ -61,6 +55,10 @@ protected:
     LoggerTest()
     {
     }
+
+    ~LoggerTest() {
+        TestLogger::reset();
+    }
 };
 
 
@@ -91,26 +89,12 @@ TEST_F(LoggerTest, GetLogger) {
     // Instantiate two loggers that should be the same
     TestLogger logger1(name1);
     TestLogger logger2(name1);
-
-    // And check they are null at this point.
-    EXPECT_FALSE(logger1.isInitialized());
-    EXPECT_FALSE(logger2.isInitialized());
-
-    // Do some random operation
-    EXPECT_TRUE(logger1.isFatalEnabled());
-    EXPECT_TRUE(logger2.isFatalEnabled());
-
-    // And check they initialized and equal
-    EXPECT_TRUE(logger1.isInitialized());
-    EXPECT_TRUE(logger2.isInitialized());
+    // And check they equal
     EXPECT_TRUE(logger1 == logger2);
 
     // Instantiate another logger with another name and check that it
     // is different to the previously instantiated ones.
     TestLogger logger3(name2);
-    EXPECT_FALSE(logger3.isInitialized());
-    EXPECT_TRUE(logger3.isFatalEnabled());
-    EXPECT_TRUE(logger3.isInitialized());
     EXPECT_FALSE(logger1 == logger3);
 }