From a57663387e4e523e8a8a4d2c3e0f10c7d41a6822 Mon Sep 17 00:00:00 2001 From: Adam Simpkins Date: Tue, 20 Jun 2017 11:02:00 -0700 Subject: [PATCH] logging: add LoggerDB::flushAllHandlers() Summary: Add a method to flush all LogHandler objects. This will be necessary to implement FB_LOG(FATAL), as we will want to flush all outstanding messages before crashing. Reviewed By: wez Differential Revision: D5189501 fbshipit-source-id: faf260b8e71e5dfed4a3b1c1aee32f072bd7b764 --- folly/experimental/logging/LogCategory.cpp | 4 ++ folly/experimental/logging/LogCategory.h | 5 +++ folly/experimental/logging/LoggerDB.cpp | 26 +++++++++++- folly/experimental/logging/LoggerDB.h | 6 +++ .../logging/test/LoggerDBTest.cpp | 40 +++++++++++++++++++ .../logging/test/TestLogHandler.h | 12 +++++- 6 files changed, 91 insertions(+), 2 deletions(-) diff --git a/folly/experimental/logging/LogCategory.cpp b/folly/experimental/logging/LogCategory.cpp index 6f02c9dc..c475bf6e 100644 --- a/folly/experimental/logging/LogCategory.cpp +++ b/folly/experimental/logging/LogCategory.cpp @@ -112,6 +112,10 @@ void LogCategory::clearHandlers() { // LogHandler destructors. } +std::vector> LogCategory::getHandlers() const { + return *(handlers_.rlock()); +} + void LogCategory::setLevel(LogLevel level, bool inherit) { // We have to set the level through LoggerDB, since we require holding // the LoggerDB lock to iterate through our children in case our effective diff --git a/folly/experimental/logging/LogCategory.h b/folly/experimental/logging/LogCategory.h index e589400e..d7989f4b 100644 --- a/folly/experimental/logging/LogCategory.h +++ b/folly/experimental/logging/LogCategory.h @@ -150,6 +150,11 @@ class LogCategory { */ void clearHandlers(); + /** + * Get the list of LogHandlers attached to this category. + */ + std::vector> getHandlers() const; + /* Internal methods for use by other parts of the logging library code */ /** diff --git a/folly/experimental/logging/LoggerDB.cpp b/folly/experimental/logging/LoggerDB.cpp index 0d643451..4c8d3a80 100644 --- a/folly/experimental/logging/LoggerDB.cpp +++ b/folly/experimental/logging/LoggerDB.cpp @@ -15,10 +15,13 @@ */ #include +#include + #include #include #include #include +#include #include #include #include @@ -40,7 +43,8 @@ class LoggerDBSingleton { // // However, we do call db_->cleanupHandlers() to destroy any registered // LogHandler objects. The LogHandlers can be user-defined objects and may - // hold resources that should be cleaned up. + // hold resources that should be cleaned up. This also ensures that the + // LogHandlers flush all outstanding messages before we exit. db_->cleanupHandlers(); } @@ -174,6 +178,26 @@ void LoggerDB::cleanupHandlers() { } } +void LoggerDB::flushAllHandlers() { + // Build a set of all LogHandlers. We use a set to avoid calling flush() + // more than once on the same handler if it is registered on multiple + // different categories. + std::set> handlers; + { + auto loggersByName = loggersByName_.wlock(); + for (const auto& entry : *loggersByName) { + for (const auto& handler : entry.second->getHandlers()) { + handlers.emplace(handler); + } + } + } + + // Call flush() on each handler + for (const auto& handler : handlers) { + handler->flush(); + } +} + LogLevel LoggerDB::xlogInit( StringPiece categoryName, std::atomic* xlogCategoryLevel, diff --git a/folly/experimental/logging/LoggerDB.h b/folly/experimental/logging/LoggerDB.h index b1a1f249..9580b743 100644 --- a/folly/experimental/logging/LoggerDB.h +++ b/folly/experimental/logging/LoggerDB.h @@ -91,6 +91,12 @@ class LoggerDB { */ void cleanupHandlers(); + /** + * Call flush() on all LogHandler objects registered on any LogCategory in + * this LoggerDB. + */ + void flushAllHandlers(); + /** * Initialize the LogCategory* and std::atomic used by an XLOG() * statement. diff --git a/folly/experimental/logging/test/LoggerDBTest.cpp b/folly/experimental/logging/test/LoggerDBTest.cpp index 181214c3..d7fbf543 100644 --- a/folly/experimental/logging/test/LoggerDBTest.cpp +++ b/folly/experimental/logging/test/LoggerDBTest.cpp @@ -15,6 +15,7 @@ */ #include #include +#include #include using namespace folly; @@ -34,6 +35,45 @@ TEST(LoggerDB, getCategory) { LoggerDB db{LoggerDB::TESTING}; } +TEST(LoggerDB, flushAllHandlers) { + LoggerDB db{LoggerDB::TESTING}; + auto* cat1 = db.getCategory("foo"); + auto* cat2 = db.getCategory("foo.bar.test"); + auto* cat3 = db.getCategory("hello.world"); + auto* cat4 = db.getCategory("other.category"); + + auto h1 = std::make_shared(); + auto h2 = std::make_shared(); + auto h3 = std::make_shared(); + + cat1->addHandler(h1); + + cat2->addHandler(h2); + cat2->addHandler(h3); + + cat3->addHandler(h1); + cat3->addHandler(h2); + cat3->addHandler(h3); + + cat4->addHandler(h1); + + EXPECT_EQ(0, h1->getFlushCount()); + EXPECT_EQ(0, h2->getFlushCount()); + EXPECT_EQ(0, h3->getFlushCount()); + + // Calling flushAllHandlers() should only flush each handler once, + // even when they are attached to multiple categories. + db.flushAllHandlers(); + EXPECT_EQ(1, h1->getFlushCount()); + EXPECT_EQ(1, h2->getFlushCount()); + EXPECT_EQ(1, h3->getFlushCount()); + + db.flushAllHandlers(); + EXPECT_EQ(2, h1->getFlushCount()); + EXPECT_EQ(2, h2->getFlushCount()); + EXPECT_EQ(2, h3->getFlushCount()); +} + TEST(LoggerDB, processConfigString) { LoggerDB db{LoggerDB::TESTING}; db.processConfigString("foo.bar=dbg5"); diff --git a/folly/experimental/logging/test/TestLogHandler.h b/folly/experimental/logging/test/TestLogHandler.h index ae74313b..32cba46b 100644 --- a/folly/experimental/logging/test/TestLogHandler.h +++ b/folly/experimental/logging/test/TestLogHandler.h @@ -25,6 +25,9 @@ namespace folly { /** * A LogHandler that simply keeps a vector of all LogMessages it receives. + * + * This class is not thread-safe. It is intended to be used in single-threaded + * tests. */ class TestLogHandler : public LogHandler { public: @@ -38,9 +41,16 @@ class TestLogHandler : public LogHandler { messages_.emplace_back(message, handlerCategory); } - void flush() override {} + void flush() override { + ++flushCount_; + } + + uint64_t getFlushCount() const { + return flushCount_; + } private: std::vector> messages_; + uint64_t flushCount_{0}; }; } -- 2.34.1