core: Make logger thread-safe
Change-Id: Ic45dc0912da9c2e3f074d3f74921bfe33741bf63
Refs: #2489
diff --git a/core/logger-factory.cpp b/core/logger-factory.cpp
index 483387f..20578c3 100644
--- a/core/logger-factory.cpp
+++ b/core/logger-factory.cpp
@@ -25,6 +25,8 @@
#include "logger-factory.hpp"
+#include <boost/range/adaptor/map.hpp>
+
#ifdef HAVE_CUSTOM_LOGGER
#error "This file should not be compiled when custom logger is used"
#endif
@@ -70,44 +72,37 @@
// std::cerr << m_levelNames.begin()->first << std::endl;
LevelMap::const_iterator levelIt = m_levelNames.find(upperLevel);
- if (levelIt != m_levelNames.end())
- {
- return levelIt->second;
- }
- try
- {
- uint32_t levelNo = boost::lexical_cast<uint32_t>(level);
+ if (levelIt != m_levelNames.end()) {
+ return levelIt->second;
+ }
+ try {
+ uint32_t levelNo = boost::lexical_cast<uint32_t>(level);
- if ((boost::lexical_cast<uint32_t>(LOG_NONE) <= levelNo &&
- levelNo <= boost::lexical_cast<uint32_t>(LOG_TRACE)) ||
- levelNo == LOG_ALL)
- {
- return static_cast<LogLevel>(levelNo);
- }
+ if ((boost::lexical_cast<uint32_t>(LOG_NONE) <= levelNo &&
+ levelNo <= boost::lexical_cast<uint32_t>(LOG_TRACE)) ||
+ levelNo == LOG_ALL) {
+ return static_cast<LogLevel>(levelNo);
}
- catch (const boost::bad_lexical_cast& error)
- {
- }
- throw LoggerFactory::Error("Unsupported logging level \"" +
- level + "\"");
+ }
+ catch (const boost::bad_lexical_cast& error) {
+ }
+
+ throw LoggerFactory::Error("Unsupported logging level \"" + level + "\"");
}
LogLevel
LoggerFactory::extractLevel(const ConfigSection& item, const std::string& key)
{
std::string levelString;
- try
- {
- levelString = item.get_value<std::string>();
- }
- catch (const boost::property_tree::ptree_error& error)
- {
- }
+ try {
+ levelString = item.get_value<std::string>();
+ }
+ catch (const boost::property_tree::ptree_error& error) {
+ }
- if (levelString.empty())
- {
- throw LoggerFactory::Error("No logging level found for option \"" + key + "\"");
- }
+ if (levelString.empty()) {
+ throw LoggerFactory::Error("No logging level found for option \"" + key + "\"");
+ }
return parseLevel(levelString);
}
@@ -131,58 +126,51 @@
// Forwarder WARN
// }
- if (!isDryRun)
- {
- ConfigSection::const_assoc_iterator item = section.find("default_level");
- if (item != section.not_found())
- {
- LogLevel level = extractLevel(item->second, "default_level");
- setDefaultLevel(level);
- }
- else
- {
- setDefaultLevel(LOG_INFO);
- }
+ if (!isDryRun) {
+ ConfigSection::const_assoc_iterator item = section.find("default_level");
+ if (item != section.not_found()) {
+ LogLevel level = extractLevel(item->second, "default_level");
+ setDefaultLevel(level);
}
-
- for (ConfigSection::const_iterator item = section.begin();
- item != section.end();
- ++item)
- {
- LogLevel level = extractLevel(item->second, item->first);
-
- if (item->first == "default_level")
- {
- // do nothing
- }
- else
- {
- LoggerMap::iterator loggerIt = m_loggers.find(item->first);
- if (loggerIt == m_loggers.end())
- {
- NFD_LOG_DEBUG("Failed to configure logging level for module \"" <<
- item->first << "\" (module not found)");
- }
- else if (!isDryRun)
- {
- // std::cerr << "changing level for module " << item->first << " to " << level << std::endl;
- loggerIt->second.setLogLevel(level);
- }
- }
+ else {
+ setDefaultLevel(LOG_INFO);
}
+ }
+
+ for (const auto& i : section) {
+ LogLevel level = extractLevel(i.second, i.first);
+
+ if (i.first == "default_level") {
+ // do nothing
+ }
+ else {
+ std::unique_lock<std::mutex> lock(m_loggersGuard);
+ LoggerMap::iterator loggerIt = m_loggers.find(i.first);
+ if (loggerIt == m_loggers.end()) {
+ lock.unlock();
+ NFD_LOG_DEBUG("Failed to configure logging level for module \"" <<
+ i.first << "\" (module not found)");
+ }
+ else if (!isDryRun) {
+ loggerIt->second.setLogLevel(level);
+ lock.unlock();
+ NFD_LOG_DEBUG("Changing level for module " << i.first << " to " << level);
+ }
+ }
+ }
}
void
LoggerFactory::setDefaultLevel(LogLevel level)
{
// std::cerr << "changing to default_level " << level << std::endl;
+ std::lock_guard<std::mutex> lock(m_loggersGuard);
m_defaultLevel = level;
- for (LoggerMap::iterator i = m_loggers.begin(); i != m_loggers.end(); ++i)
- {
- // std::cerr << "changing " << i->first << " to default " << m_defaultLevel << std::endl;
- i->second.setLogLevel(m_defaultLevel);
- }
+ for (auto&& logger : m_loggers) {
+ // std::cerr << "changing " << i->first << " to default " << m_defaultLevel << std::endl;
+ logger.second.setLogLevel(m_defaultLevel);
+ }
}
Logger&
@@ -197,6 +185,8 @@
// std::cerr << "creating logger for " << moduleName
// << " with level " << m_defaultLevel << std::endl;
+ std::lock_guard<std::mutex> lock(m_loggersGuard);
+
std::pair<LoggerMap::iterator, bool> loggerIt =
m_loggers.insert(NameAndLogger(moduleName, Logger(moduleName, m_defaultLevel)));
@@ -206,11 +196,12 @@
std::list<std::string>
LoggerFactory::getModules() const
{
+ std::lock_guard<std::mutex> lock(m_loggersGuard);
+
std::list<std::string> modules;
- for (LoggerMap::const_iterator i = m_loggers.begin(); i != m_loggers.end(); ++i)
- {
- modules.push_back(i->first);
- }
+ for (const auto& loggerName : m_loggers | boost::adaptors::map_keys) {
+ modules.push_back(loggerName);
+ }
return modules;
}
diff --git a/core/logger-factory.hpp b/core/logger-factory.hpp
index 9f4a9f3..606ac1c 100644
--- a/core/logger-factory.hpp
+++ b/core/logger-factory.hpp
@@ -101,6 +101,7 @@
typedef std::pair<std::string, Logger> NameAndLogger;
LoggerMap m_loggers;
+ mutable std::mutex m_loggersGuard;
LogLevel m_defaultLevel;
};
diff --git a/core/logger.cpp b/core/logger.cpp
index 5ba8c39..3a9da61 100644
--- a/core/logger.cpp
+++ b/core/logger.cpp
@@ -36,6 +36,8 @@
namespace nfd {
+std::mutex g_logMutex;
+
Logger::Logger(const std::string& name, LogLevel level)
: m_moduleName(name)
, m_enabledLogLevel(level)
diff --git a/core/logger.hpp b/core/logger.hpp
index 6d01fa0..54af7cb 100644
--- a/core/logger.hpp
+++ b/core/logger.hpp
@@ -32,6 +32,8 @@
#include "custom-logger.hpp"
#else
+#include <mutex>
+
namespace nfd {
/** \brief indicates a log level
@@ -128,12 +130,15 @@
template<> \
nfd::Logger& cls<s1, s2>::g_logger = nfd::LoggerFactory::create(name)
+extern std::mutex g_logMutex;
#define NFD_LOG(level, msg, expression) \
do { \
- if (g_logger.isEnabled(::nfd::LOG_##level)) \
+ if (g_logger.isEnabled(::nfd::LOG_##level)) { \
+ std::lock_guard<std::mutex> lock(::nfd::g_logMutex); \
std::clog << ::nfd::Logger::now() << " "#msg": " \
<< "[" << g_logger << "] " << expression << "\n"; \
+ } \
} while (false)
#define NFD_LOG_TRACE(expression) NFD_LOG(TRACE, TRACE, expression)
diff --git a/tests/core/logger.cpp b/tests/core/logger.cpp
index 9ae7fa4..50df279 100644
--- a/tests/core/logger.cpp
+++ b/tests/core/logger.cpp
@@ -864,6 +864,24 @@
}
}
+BOOST_FIXTURE_TEST_CASE(LoggerFactoryListModules, LoggerFixture)
+{
+ std::set<std::string> testCaseLoggers{"LoggerFactoryListModules1", "LoggerFactoryListModules2"};
+
+ for (const auto& name : testCaseLoggers) {
+ LoggerFactory::create(name);
+ }
+
+ auto&& modules = LoggerFactory::getInstance().getModules();
+ BOOST_CHECK_GE(modules.size(), 2);
+
+ for (const auto& name : modules) {
+ testCaseLoggers.erase(name);
+ }
+
+ BOOST_CHECK_EQUAL(testCaseLoggers.size(), 0);
+}
+
BOOST_AUTO_TEST_SUITE_END()
} // namespace tests