[2445] Merge branch 'trac2445' of ssh://git.bind10.isc.org/var/bind10/git/bind10 into trac2445

src/lib/log/Makefile.am

@@ -31,7 +31,7 @@ libb10_log_la_SOURCES += message_initializer.cc message_initializer.h
 libb10_log_la_SOURCES += message_reader.cc message_reader.h
 libb10_log_la_SOURCES += message_types.h
 libb10_log_la_SOURCES += output_option.cc output_option.h
-libb10_log_la_SOURCES += log_buffer.cc log_buffer.h
+libb10_log_la_SOURCES += log_buffer_impl.cc log_buffer_impl.h
 
 EXTRA_DIST  = README
 EXTRA_DIST += logimpl_messages.mes

src/lib/log/README

@@ -366,11 +366,11 @@ first time a logger specification is processed. This way the program can
 use logging before even processing its logging configuration. As soon as any
 specification is processed (even an empty one), the buffered log messages will
 be flushed according to the specification. Note that if this option is used,
-the program SHOULD call one of the LoggerManager's process() calls. If the
-program exits before this is done, all log messages are dumped in a shortened
-format to stdout (so that no messages get lost). If you are using the built-in
-logging configuration handling in ModuleCCSession, this is automatically
-handled.
+the program SHOULD call one of the LoggerManager's process() calls (if you
+are using the built-in logging configuration handling in ModuleCCSession,
+this is automatically handled). If the program exits before this is done,
+all log messages are dumped in a raw format to stdout (so that no messages
+get lost).
 
 Variant #2, Used by Unit Tests
 ------------------------------

src/lib/log/log_buffer.cc → src/lib/log/log_buffer_impl.cc

@@ -12,22 +12,15 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <log/log_buffer.h>
-#include <log4cplus/loglevel.h>
+#include <log/log_buffer_impl.h>
 
+#include <log4cplus/loglevel.h>
 #include <boost/scoped_ptr.hpp>
-#include <iostream>
+#include <cstdio>
 
 namespace isc {
 namespace log {
-
-LogBuffer& getLogBuffer() {
-    static boost::scoped_ptr<LogBuffer> log_buffer(NULL);
-    if (!log_buffer) {
-        log_buffer.reset(new LogBuffer);
-    }
-    return (*log_buffer);
-}
+namespace internal {
 
 LogBuffer::~LogBuffer() {
     // If there is anything left in the buffer,
@@ -64,23 +57,24 @@ LogBuffer::flushStdout() {
     // be a good idea; as we can't reliably know whether in what
     // state the logger instance is now (or what the specific logger's
     // settings were).
-    // So we print a slightly shortened format (it really only excludes
-    // the time and the pid)
+    // So we print a raw format (it excludes the time and the pid, and
+    // it prints severity as a number)
     LoggerEventPtrList::const_iterator it;
-    const log4cplus::LogLevelManager& manager =
-        log4cplus::getLogLevelManager();
     for (it = stored_.begin(); it != stored_.end(); ++it) {
-        std::cout << manager.toString((*it)->getLogLevel()) << " " <<
-                     "[" << (*it)->getLoggerName() << "] " <<
-                     (*it)->getMessage() << std::endl;
+        std::printf("Severity=%d [%s]: %s\n", (*it)->getLogLevel(),
+                    (*it)->getLoggerName().c_str(),
+                    (*it)->getMessage().c_str());
     }
     stored_.clear();
 }
 
 void
 LogBuffer::flush() {
+    LoggerEventPtrList stored_copy;
+    stored_.swap(stored_copy);
+
     LoggerEventPtrList::const_iterator it;
-    for (it = stored_.begin(); it != stored_.end(); ++it) {
+    for (it = stored_copy.begin(); it != stored_copy.end(); ++it) {
         log4cplus::Logger logger =
             log4cplus::Logger::getInstance((*it)->getLoggerName());
 
@@ -100,5 +94,6 @@ BufferAppender::append(const log4cplus::spi::InternalLoggingEvent& event) {
     buffer_.add(event);
 }
 
+} // end namespace internal
 } // end namespace log
 } // end namespace isc

src/lib/log/log_buffer.h → src/lib/log/log_buffer_impl.h

@@ -23,6 +23,7 @@
 
 namespace isc {
 namespace log {
+namespace internal {
 
 /// \brief Buffer add after flush
 ///
@@ -84,6 +85,7 @@ public:
     /// Once this method has been called, no more events can be
     /// added through calls to \c add(); if \c add() is called after flush(),
     /// an exception will be raised.
+    /// If flush for any reason fails, the remaining events are dropped.
     void flush();
 
     /// \brief Returns number of stored events
@@ -105,27 +107,34 @@ private:
 /// This class can be set as an Appender for log4cplus loggers
 ///
 /// When logging an event, it will not actually log anything, but
-/// merely add it to the singleton LogBuffer instance
+/// merely add it to its internal LogBuffer
 class BufferAppender : public log4cplus::Appender {
 public:
     /// \brief Constructor
     ///
-    /// \note Only a reference to the LogBuffer is stored, so
-    /// this buffer must remain in scope during the lifetime of
-    /// the appender. In general, only one buffer would be needed,
-    /// and for that purpose there is the singleton instance
-    /// that can be reached using \c getLogBuffer()
-    BufferAppender(LogBuffer& buffer) : buffer_(buffer) {}
+    /// Constructs a BufferAppender with its own LogBuffer instance
+    BufferAppender() {}
     virtual void close() {}
+
+    /// \brief Flush the internal buffer
+    void flush() {
+        buffer_.flush();
+    }
+
+    /// \brief Access to the internal log buffer
+    ///
+    /// This is mostly for testing
+    LogBuffer& getLogBuffer() {
+        return (buffer_);
+    }
+
 protected:
     virtual void append(const log4cplus::spi::InternalLoggingEvent& event);
 private:
-    LogBuffer& buffer_;
+    LogBuffer buffer_;
 };
 
-/// \brief Getter for the singleton instance of the log buffer
-LogBuffer& getLogBuffer();
-
+} // end namespace internal
 } // end namespace log
 } // end namespace isc
 

src/lib/log/logger_manager_impl.cc

@@ -30,7 +30,7 @@
 #include <log/log_messages.h>
 #include <log/logger_name.h>
 #include <log/logger_specification.h>
-#include <log/log_buffer.h>
+#include <log/log_buffer_impl.h>
 
 using namespace std;
 
@@ -44,6 +44,8 @@ namespace log {
 // explicitly reset the logging severity.)
 void
 LoggerManagerImpl::processInit() {
+    storeBufferAppenders();
+
     log4cplus::Logger::getDefaultHierarchy().resetConfiguration();
     initRootLogger();
 }
@@ -51,16 +53,15 @@ LoggerManagerImpl::processInit() {
 // Flush the LogBuffer at the end of processing a new specification
 void
 LoggerManagerImpl::processEnd() {
-    getLogBuffer().flush();
+    flushBufferAppenders();
 }
 
 // Process logging specification.  Set up the common states then dispatch to
 // add output specifications.
 void
 LoggerManagerImpl::processSpecification(const LoggerSpecification& spec) {
-    log4cplus::Logger logger;
-    // If this is an 'empty' specification, just set the root logger
-    logger = log4cplus::Logger::getInstance(expandLoggerName(spec.getName()));
+    log4cplus::Logger logger = log4cplus::Logger::getInstance(
+                                   expandLoggerName(spec.getName()));
 
     // Set severity level according to specification entry.
     logger.setLogLevel(LoggerLevelImpl::convertFromBindLevel(
@@ -141,7 +142,7 @@ LoggerManagerImpl::createFileAppender(log4cplus::Logger& logger,
 
 void
 LoggerManagerImpl::createBufferAppender(log4cplus::Logger& logger) {
-    log4cplus::SharedAppenderPtr bufferapp(new BufferAppender(getLogBuffer()));
+    log4cplus::SharedAppenderPtr bufferapp(new internal::BufferAppender());
     bufferapp->setName("buffer");
     logger.addAppender(bufferapp);
     // Since we do not know at what level the loggers will end up
@@ -240,5 +241,30 @@ void LoggerManagerImpl::setSyslogAppenderLayout(
     appender->setLayout(layout);
 }
 
+void LoggerManagerImpl::storeBufferAppenders() {
+    // Walk through all loggers, and find any buffer appenders there
+    log4cplus::LoggerList loggers = log4cplus::Logger::getCurrentLoggers();
+    log4cplus::LoggerList::iterator it;
+    for (it = loggers.begin(); it != loggers.end(); ++it) {
+        log4cplus::SharedAppenderPtr buffer_appender =
+            it->getAppender("buffer");
+        if (buffer_appender) {
+            buffer_appender_store_.push_back(buffer_appender);
+        }
+    }
+}
+
+void LoggerManagerImpl::flushBufferAppenders() {
+    std::vector<log4cplus::SharedAppenderPtr> copy;
+    buffer_appender_store_.swap(copy);
+
+    std::vector<log4cplus::SharedAppenderPtr>::iterator it;
+    for (it = copy.begin(); it != copy.end(); ++it) {
+        internal::BufferAppender* app =
+            dynamic_cast<internal::BufferAppender*>(it->get());
+        app->flush();
+    }
+}
+
 } // namespace log
 } // namespace isc

src/lib/log/logger_manager_impl.h

@@ -58,7 +58,7 @@ public:
     /// This resets the hierachy of loggers back to their defaults.  This means
     /// that all non-root loggers (if they exist) are set to NOT_SET, and the
     /// root logger reset to logging informational messages.
-    static void processInit();
+    void processInit();
 
     /// \brief Process Specification
     ///
@@ -70,7 +70,7 @@ public:
     /// \brief End Processing
     ///
     /// Terminates the processing of the logging specifications.
-    static void processEnd();
+    void processEnd();
 
     /// \brief Implementation-specific initialization
     ///
@@ -136,9 +136,8 @@ private:
     /// \brief Create buffered appender
     ///
     /// Appends an object to the logger that will store the log events sent
-    /// to the logger in the singleton \c LogBuffer instance. These log
-    /// messages are replayed to the logger when the LogBuffer instance is
-    /// flushed (which is done at the end of \c ProcessSpecification().
+    /// to the logger. These log messages are replayed to the logger in
+    /// processEnd().
     ///
     /// \param logger Log4cplus logger to which the appender must be attached.
     static void createBufferAppender(log4cplus::Logger& logger);
@@ -175,6 +174,25 @@ private:
     ///
     /// \param appender Appender for which this pattern is to be set.
     static void setSyslogAppenderLayout(log4cplus::SharedAppenderPtr& appender);
+
+    /// \brief Store all buffer appenders
+    ///
+    /// When processing a new specification, this method can be used
+    /// to keep a list of the buffer appenders; the caller can then
+    /// process the specification, and call \c flushBufferAppenders()
+    /// to flush and clear the list
+    void storeBufferAppenders();
+
+    /// \brief Flush the stored buffer appenders
+    ///
+    /// This flushes the list of buffer appenders stored in
+    /// \c storeBufferAppenders(), and clears it
+    void flushBufferAppenders();
+
+    /// Only used between processInit() and processEnd(), to temporarily
+    /// store the buffer appenders in order to flush them after
+    /// processSpecification() calls have been completed
+    std::vector<log4cplus::SharedAppenderPtr> buffer_appender_store_;
 };
 
 } // namespace log

src/lib/log/tests/buffer_logger_test.sh.in

@@ -46,9 +46,9 @@ passfail $?
 
 echo -n  "   - Buffer excluding process() call: "
 cat > $tempfile << .
-INFO [buffertest.log] LOG_BAD_SEVERITY unrecognized log severity: info
-DEBUG [buffertest.log] LOG_BAD_DESTINATION unrecognized log destination: debug-50
-INFO [buffertest.log] LOG_BAD_SEVERITY unrecognized log severity: info
+Severity=20000 [buffertest.log]: LOG_BAD_SEVERITY unrecognized log severity: info
+Severity=10000 [buffertest.log]: LOG_BAD_DESTINATION unrecognized log destination: debug-50
+Severity=20000 [buffertest.log]: LOG_BAD_SEVERITY unrecognized log severity: info
 .
 ./buffer_logger_test -n 2>&1 | diff $tempfile -
 passfail $?

src/lib/log/tests/log_buffer_unittest.cc

@@ -18,7 +18,7 @@
 #include <log/macros.h>
 #include <log/logger_support.h>
 #include <log/log_messages.h>
-#include <log/log_buffer.h>
+#include <log/log_buffer_impl.h>
 
 #include <log4cplus/loggingmacros.h>
 #include <log4cplus/logger.h>
@@ -26,14 +26,17 @@
 #include <log4cplus/spi/loggingevent.h>
 
 using namespace isc::log;
+using namespace isc::log::internal;
 
 namespace isc {
 namespace log {
 
 class LogBufferTest : public ::testing::Test {
 protected:
-    LogBufferTest() : appender1(new BufferAppender(buffer1)),
-                      appender2(new BufferAppender(buffer2)),
+    LogBufferTest() : buffer_appender1(new BufferAppender()),
+                      buffer_appender2(new BufferAppender()),
+                      appender1(buffer_appender1),
+                      appender2(buffer_appender2),
                       logger(log4cplus::Logger::getInstance("buffer"))
     {
         logger.setLogLevel(log4cplus::TRACE_LOG_LEVEL);
@@ -50,12 +53,14 @@ protected:
             new log4cplus::NullAppender());
         logger.removeAllAppenders();
         logger.addAppender(null_appender);
-        buffer1.flush();
-        buffer2.flush();
+        buffer_appender1->flush();
+        buffer_appender2->flush();
     }
 
-    LogBuffer buffer1;
-    LogBuffer buffer2;
+    //LogBuffer buffer_appender1->getLogBuffer().
+    //LogBuffer buffer_appender2->getLogBuffer().
+    BufferAppender* buffer_appender1;
+    BufferAppender* buffer_appender2;
     log4cplus::SharedAppenderPtr appender1;
     log4cplus::SharedAppenderPtr appender2;
     log4cplus::Logger logger;
@@ -64,43 +69,43 @@ protected:
 // Test that log events are indeed stored, and that they are
 // flushed to the new appenders of their logger
 TEST_F(LogBufferTest, flush) {
-    ASSERT_EQ(0, buffer1.getBufferSize());
-    ASSERT_EQ(0, buffer2.getBufferSize());
+    ASSERT_EQ(0, buffer_appender1->getLogBuffer().getBufferSize());
+    ASSERT_EQ(0, buffer_appender2->getLogBuffer().getBufferSize());
 
     // Create a Logger, log a few messages with the first appender
     logger.addAppender(appender1);
     LOG4CPLUS_INFO(logger, "Foo");
-    ASSERT_EQ(1, buffer1.getBufferSize());
+    ASSERT_EQ(1, buffer_appender1->getLogBuffer().getBufferSize());
     LOG4CPLUS_INFO(logger, "Foo");
-    ASSERT_EQ(2, buffer1.getBufferSize());
+    ASSERT_EQ(2, buffer_appender1->getLogBuffer().getBufferSize());
     LOG4CPLUS_INFO(logger, "Foo");
-    ASSERT_EQ(3, buffer1.getBufferSize());
+    ASSERT_EQ(3, buffer_appender1->getLogBuffer().getBufferSize());
 
     // Second buffer should still be empty
-    ASSERT_EQ(0, buffer2.getBufferSize());
+    ASSERT_EQ(0, buffer_appender2->getLogBuffer().getBufferSize());
 
     // Replace the appender by the second one, and call flush;
     // this should cause all events to be moved to the second buffer
     logger.removeAllAppenders();
     logger.addAppender(appender2);
-    buffer1.flush();
-    ASSERT_EQ(0, buffer1.getBufferSize());
-    ASSERT_EQ(3, buffer2.getBufferSize());
+    buffer_appender1->flush();
+    ASSERT_EQ(0, buffer_appender1->getLogBuffer().getBufferSize());
+    ASSERT_EQ(3, buffer_appender2->getLogBuffer().getBufferSize());
 }
 
 // Once flushed, logging new messages with the same buffer should fail
 TEST_F(LogBufferTest, addAfterFlush) {
     logger.addAppender(appender1);
-    buffer1.flush();
+    buffer_appender1->flush();
     EXPECT_THROW(LOG4CPLUS_INFO(logger, "Foo"), LogBufferAddAfterFlush);
     // It should not have been added
-    ASSERT_EQ(0, buffer1.getBufferSize());
+    ASSERT_EQ(0, buffer_appender1->getLogBuffer().getBufferSize());
 
     // But logging should work again as long as a different buffer is used
     logger.removeAllAppenders();
     logger.addAppender(appender2);
     LOG4CPLUS_INFO(logger, "Foo");
-    ASSERT_EQ(1, buffer2.getBufferSize());
+    ASSERT_EQ(1, buffer_appender2->getLogBuffer().getBufferSize());
 }
 
 TEST_F(LogBufferTest, addDirectly) {
@@ -108,24 +113,24 @@ TEST_F(LogBufferTest, addDirectly) {
     log4cplus::spi::InternalLoggingEvent event("buffer",
                                                log4cplus::INFO_LOG_LEVEL,
                                                "Bar", "file", 123);
-    buffer1.add(event);
-    ASSERT_EQ(1, buffer1.getBufferSize());
+    buffer_appender1->getLogBuffer().add(event);
+    ASSERT_EQ(1, buffer_appender1->getLogBuffer().getBufferSize());
 
     // Do one from a smaller scope to make sure destruction doesn't harm
     {
         log4cplus::spi::InternalLoggingEvent event2("buffer",
                                                     log4cplus::INFO_LOG_LEVEL,
                                                     "Bar", "file", 123);
-        buffer1.add(event2);
+        buffer_appender1->getLogBuffer().add(event2);
     }
-    ASSERT_EQ(2, buffer1.getBufferSize());
+    ASSERT_EQ(2, buffer_appender1->getLogBuffer().getBufferSize());
 
     // And flush them to the next
     logger.removeAllAppenders();
     logger.addAppender(appender2);
-    buffer1.flush();
-    ASSERT_EQ(0, buffer1.getBufferSize());
-    ASSERT_EQ(2, buffer2.getBufferSize());
+    buffer_appender1->flush();
+    ASSERT_EQ(0, buffer_appender1->getLogBuffer().getBufferSize());
+    ASSERT_EQ(2, buffer_appender2->getLogBuffer().getBufferSize());
 }
 
 }