[2445] Remove LogBuffer singleton

LogBuffers are now members of BufferAppender instances, and the singleton has been removed. LoggerManagerImpl keeps them (if any) around while processing a new specification, and flushes them when done.

src/lib/log/log_buffer_impl.cc

@@ -22,14 +22,6 @@ namespace isc {
 namespace log {
 namespace internal {
 
-LogBuffer& getLogBuffer() {
-    static boost::scoped_ptr<LogBuffer> log_buffer(NULL);
-    if (!log_buffer) {
-        log_buffer.reset(new LogBuffer);
-    }
-    return (*log_buffer);
-}
-
 LogBuffer::~LogBuffer() {
     // If there is anything left in the buffer,
     // it means no reconfig has been done, and

src/lib/log/log_buffer_impl.h

@@ -107,27 +107,33 @@ 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

@@ -44,6 +44,8 @@ namespace log {
 // explicitly reset the logging severity.)
 void
 LoggerManagerImpl::processInit() {
+    storeBufferAppenders();
+
     log4cplus::Logger::getDefaultHierarchy().resetConfiguration();
     initRootLogger();
 }
@@ -51,7 +53,7 @@ LoggerManagerImpl::processInit() {
 // Flush the LogBuffer at the end of processing a new specification
 void
 LoggerManagerImpl::processEnd() {
-    internal::getLogBuffer().flush();
+    flushBufferAppenders();
 }
 
 // Process logging specification.  Set up the common states then dispatch to
@@ -140,8 +142,7 @@ LoggerManagerImpl::createFileAppender(log4cplus::Logger& logger,
 
 void
 LoggerManagerImpl::createBufferAppender(log4cplus::Logger& logger) {
-    log4cplus::SharedAppenderPtr bufferapp(
-        new internal::BufferAppender(internal::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/log_buffer_unittest.cc

@@ -33,8 +33,10 @@ 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);
@@ -51,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;
@@ -65,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) {
@@ -109,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());
 }
 
 }