Stop all file watch dog instances in LogManager::shutdown(). (#121)
This PR aims to address LOG4CXX-451.
Also included is support for automatic start of a watchdog when using automatic configuration by providing a positive integer value in the LOG4CXX_CONFIGURATION_WATCH_SECONDS environment variable.
diff --git a/src/main/cpp/aprinitializer.cpp b/src/main/cpp/aprinitializer.cpp
index 8831412..6e4214c 100644
--- a/src/main/cpp/aprinitializer.cpp
+++ b/src/main/cpp/aprinitializer.cpp
@@ -46,6 +46,7 @@
apr_initialize();
apr_pool_create(&p, NULL);
apr_atomic_init(p);
+ apr_atomic_init(p);
startTime = apr_time_now();
#if APR_HAS_THREADS
apr_status_t stat = apr_threadkey_private_create(&tlsKey, tlsDestruct, p);
@@ -56,25 +57,32 @@
APRInitializer::~APRInitializer()
{
- {
+ stopWatchDogs();
+ isDestructed = true;
#if APR_HAS_THREADS
- std::unique_lock<std::mutex> lock(mutex);
- apr_threadkey_private_delete(tlsKey);
-#endif
-
- for (std::list<FileWatchdog*>::iterator iter = watchdogs.begin();
- iter != watchdogs.end();
- iter++)
- {
- delete *iter;
- }
- }
-
- // TODO LOGCXX-322
-#ifndef APR_HAS_THREADS
+ std::unique_lock<std::mutex> lock(mutex);
+ apr_threadkey_private_delete(tlsKey);
+#else
apr_terminate();
#endif
- isDestructed = true;
+}
+
+void APRInitializer::stopWatchDogs()
+{
+#if APR_HAS_THREADS
+ std::unique_lock<std::mutex> lock(mutex);
+#endif
+
+ while (!watchdogs.empty())
+ {
+ delete watchdogs.back();
+ watchdogs.pop_back();
+ }
+}
+
+void APRInitializer::unregisterAll()
+{
+ getInstance().stopWatchDogs();
}
APRInitializer& APRInitializer::getInstance()
@@ -132,7 +140,7 @@
#if APR_HAS_THREADS
std::unique_lock<std::mutex> lock(this->mutex);
#endif
- this->objects[key] = pObject;
+ this->objects[key] = pObject;
}
const ObjectPtr& APRInitializer::findOrAddObject(size_t key, std::function<ObjectPtr()> creator)
@@ -140,8 +148,8 @@
#if APR_HAS_THREADS
std::unique_lock<std::mutex> lock(this->mutex);
#endif
- auto pItem = this->objects.find(key);
- if (this->objects.end() == pItem)
- pItem = this->objects.emplace(key, creator()).first;
- return pItem->second;
+ auto pItem = this->objects.find(key);
+ if (this->objects.end() == pItem)
+ pItem = this->objects.emplace(key, creator()).first;
+ return pItem->second;
}
diff --git a/src/main/cpp/defaultconfigurator.cpp b/src/main/cpp/defaultconfigurator.cpp
index 4b64298..97a594b 100644
--- a/src/main/cpp/defaultconfigurator.cpp
+++ b/src/main/cpp/defaultconfigurator.cpp
@@ -67,7 +67,8 @@
OptionConverter::selectAndConfigure(
configuration,
configuratorClassName,
- repo);
+ repo,
+ getConfigurationWatchDelay());
}
else
{
@@ -113,5 +114,16 @@
}
+int DefaultConfigurator::getConfigurationWatchDelay()
+{
+ static const LogString LOG4CXX_DEFAULT_CONFIGURATION_WATCH_KEY(LOG4CXX_STR("LOG4CXX_CONFIGURATION_WATCH_SECONDS"));
+ LogString optionStr = OptionConverter::getSystemProperty(LOG4CXX_DEFAULT_CONFIGURATION_WATCH_KEY, LogString());
+ int milliseconds = 0;
+ if (!optionStr.empty())
+ milliseconds = stoi(optionStr) * 1000;
+ return milliseconds;
+}
+
+
diff --git a/src/main/cpp/filewatchdog.cpp b/src/main/cpp/filewatchdog.cpp
index 264c1eb..7f74de3 100644
--- a/src/main/cpp/filewatchdog.cpp
+++ b/src/main/cpp/filewatchdog.cpp
@@ -24,6 +24,7 @@
#include <log4cxx/helpers/transcoder.h>
#include <log4cxx/helpers/exception.h>
#include <log4cxx/helpers/threadutility.h>
+#include <log4cxx/helpers/stringhelper.h>
#include <functional>
using namespace log4cxx;
@@ -103,6 +104,12 @@
void FileWatchdog::run()
{
+ LogString msg(LOG4CXX_STR("Checking ["));
+ msg += m_priv->file.getPath();
+ msg += LOG4CXX_STR("] at ");
+ StringHelper::toString((int)m_priv->delay, m_priv->pool, msg);
+ msg += LOG4CXX_STR(" ms interval");
+ LogLog::debug(msg);
while (m_priv->interrupted != 0xFFFF)
{
@@ -113,6 +120,10 @@
checkAndConfigure();
}
+ LogString msg2(LOG4CXX_STR("Stop checking ["));
+ msg2 += m_priv->file.getPath();
+ msg2 += LOG4CXX_STR("]");
+ LogLog::debug(msg2);
}
void FileWatchdog::start()
diff --git a/src/main/cpp/logmanager.cpp b/src/main/cpp/logmanager.cpp
index ca21d06..bbd0387 100644
--- a/src/main/cpp/logmanager.cpp
+++ b/src/main/cpp/logmanager.cpp
@@ -200,6 +200,7 @@
void LogManager::shutdown()
{
+ APRInitializer::unregisterAll();
LoggerRepositoryPtr repPtr = getLoggerRepository();
getLoggerRepository()->shutdown();
}
diff --git a/src/main/cpp/optionconverter.cpp b/src/main/cpp/optionconverter.cpp
index 58605d8..9a4b4bc 100644
--- a/src/main/cpp/optionconverter.cpp
+++ b/src/main/cpp/optionconverter.cpp
@@ -36,6 +36,41 @@
#include <log4cxx/helpers/transcoder.h>
#include <log4cxx/file.h>
#include <log4cxx/xml/domconfigurator.h>
+#include <log4cxx/logmanager.h>
+#include <apr_general.h>
+#if !defined(LOG4CXX)
+ #define LOG4CXX 1
+#endif
+#include <log4cxx/helpers/aprinitializer.h>
+
+#if APR_HAS_THREADS
+#include <log4cxx/helpers/filewatchdog.h>
+namespace log4cxx
+{
+
+class ConfiguratorWatchdog : public helpers::FileWatchdog
+{
+ spi::ConfiguratorPtr m_config;
+ public:
+ ConfiguratorWatchdog(const spi::ConfiguratorPtr& config, const File& filename)
+ : helpers::FileWatchdog(filename)
+ , m_config(config)
+ {
+ }
+
+ /**
+ Call PropertyConfigurator#doConfigure(const String& configFileName,
+ const spi::LoggerRepositoryPtr& hierarchy) with the
+ <code>filename</code> to reconfigure log4cxx.
+ */
+ void doOnChange()
+ {
+ m_config->doConfigure(file(), LogManager::getLoggerRepository());
+ }
+};
+
+}
+#endif
using namespace log4cxx;
using namespace log4cxx::helpers;
@@ -375,7 +410,7 @@
}
void OptionConverter::selectAndConfigure(const File& configFileName,
- const LogString& _clazz, spi::LoggerRepositoryPtr hierarchy)
+ const LogString& _clazz, spi::LoggerRepositoryPtr hierarchy, int delay)
{
ConfiguratorPtr configurator;
LogString clazz = _clazz;
@@ -410,5 +445,15 @@
configurator = ConfiguratorPtr(new PropertyConfigurator());
}
- configurator->doConfigure(configFileName, hierarchy);
+#if APR_HAS_THREADS
+ if (0 < delay)
+ {
+ auto dog = new ConfiguratorWatchdog(configurator, configFileName);
+ APRInitializer::registerCleanup(dog);
+ dog->setDelay(delay);
+ dog->start();
+ }
+ else
+#endif
+ configurator->doConfigure(configFileName, hierarchy);
}
diff --git a/src/main/include/log4cxx/defaultconfigurator.h b/src/main/include/log4cxx/defaultconfigurator.h
index 77cf6b0..d5d3f55 100644
--- a/src/main/include/log4cxx/defaultconfigurator.h
+++ b/src/main/include/log4cxx/defaultconfigurator.h
@@ -47,6 +47,7 @@
private:
static const LogString getConfigurationFileName();
static const LogString getConfiguratorClass();
+ static int getConfigurationWatchDelay();
diff --git a/src/main/include/log4cxx/helpers/aprinitializer.h b/src/main/include/log4cxx/helpers/aprinitializer.h
index c6edab0..4b2e501 100644
--- a/src/main/include/log4cxx/helpers/aprinitializer.h
+++ b/src/main/include/log4cxx/helpers/aprinitializer.h
@@ -55,6 +55,7 @@
*/
static void registerCleanup(FileWatchdog* watchdog);
static void unregisterCleanup(FileWatchdog* watchdog);
+ static void unregisterAll();
/**
* Store a single instance type ObjectPtr for deletion prior to termination
*/
@@ -78,6 +79,7 @@
private: // Modifiers
void addObject(size_t key, const ObjectPtr& pObject);
const ObjectPtr& findOrAddObject(size_t key, std::function<ObjectPtr()> creator);
+ void stopWatchDogs();
private: // Attributes
apr_pool_t* p;
std::mutex mutex;
diff --git a/src/main/include/log4cxx/helpers/optionconverter.h b/src/main/include/log4cxx/helpers/optionconverter.h
index f3304fd..4b20ac6 100644
--- a/src/main/include/log4cxx/helpers/optionconverter.h
+++ b/src/main/include/log4cxx/helpers/optionconverter.h
@@ -153,9 +153,12 @@
filename pointed to by <code>configFileName</code> ends in '.xml',
in which case DOMConfigurator is used.
@param hierarchy The Hierarchy to act on.
+ @param delay If greater than zero, the milliseconds to sleep
+ between checking if <code>configFileName</code> has been modified
+ and needs to be reloaded.
*/
static void selectAndConfigure(const File& configFileName,
- const LogString& clazz, spi::LoggerRepositoryPtr hierarchy);
+ const LogString& clazz, spi::LoggerRepositoryPtr hierarchy, int delay = 0);
};
} // namespace helpers
} // namespace log4cxx
diff --git a/src/test/cpp/CMakeLists.txt b/src/test/cpp/CMakeLists.txt
index a7758d3..a5e4e91 100644
--- a/src/test/cpp/CMakeLists.txt
+++ b/src/test/cpp/CMakeLists.txt
@@ -124,7 +124,7 @@
)
elseif(${testName} STREQUAL autoconfiguretestcase)
set_tests_properties(autoconfiguretestcase PROPERTIES
- ENVIRONMENT "LOG4CXX_CONFIGURATION=${UNIT_TEST_WORKING_DIR}/input/autoConfigureTest.properties;PATH=${ESCAPED_PATH}"
+ ENVIRONMENT "LOG4CXX_CONFIGURATION=${UNIT_TEST_WORKING_DIR}/input/autoConfigureTest.properties;LOG4CXX_CONFIGURATION_WATCH_SECONDS=1;PATH=${ESCAPED_PATH}"
)
else()
set_tests_properties(${testName} PROPERTIES
diff --git a/src/test/cpp/autoconfiguretestcase.cpp b/src/test/cpp/autoconfiguretestcase.cpp
index 4953e2e..9954666 100644
--- a/src/test/cpp/autoconfiguretestcase.cpp
+++ b/src/test/cpp/autoconfiguretestcase.cpp
@@ -47,6 +47,7 @@
LOGUNIT_TEST_SUITE(AutoConfigureTestCase);
LOGUNIT_TEST_THREADS(test1, 4);
LOGUNIT_TEST(test2);
+ LOGUNIT_TEST(stop);
LOGUNIT_TEST_SUITE_END();
#ifdef _DEBUG
struct Fixture
@@ -73,6 +74,11 @@
LOGUNIT_ASSERT(debugLogger);
LOGUNIT_ASSERT(debugLogger->isDebugEnabled());
}
+
+ void stop()
+ {
+ LogManager::shutdown();
+ }
};
LOGUNIT_TEST_SUITE_REGISTRATION(AutoConfigureTestCase);
