You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by sw...@apache.org on 2022/09/06 11:00:53 UTC
[logging-log4cxx] branch next_stable updated: Stop all file watch dog instances in LogManager::shutdown(). (#121)
This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch next_stable
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
The following commit(s) were added to refs/heads/next_stable by this push:
new 1dff96c7 Stop all file watch dog instances in LogManager::shutdown(). (#121)
1dff96c7 is described below
commit 1dff96c7b194b943f69e3aca34a2615a3607da5a
Author: Stephen Webb <st...@ieee.org>
AuthorDate: Tue Sep 6 21:00:48 2022 +1000
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.
---
src/main/cpp/aprinitializer.cpp | 46 +++++++++++---------
src/main/cpp/defaultconfigurator.cpp | 14 ++++++-
src/main/cpp/filewatchdog.cpp | 11 +++++
src/main/cpp/logmanager.cpp | 1 +
src/main/cpp/optionconverter.cpp | 49 +++++++++++++++++++++-
src/main/include/log4cxx/defaultconfigurator.h | 1 +
src/main/include/log4cxx/helpers/aprinitializer.h | 2 +
src/main/include/log4cxx/helpers/optionconverter.h | 5 ++-
src/test/cpp/CMakeLists.txt | 2 +-
src/test/cpp/autoconfiguretestcase.cpp | 6 +++
10 files changed, 113 insertions(+), 24 deletions(-)
diff --git a/src/main/cpp/aprinitializer.cpp b/src/main/cpp/aprinitializer.cpp
index 88314125..6e4214c4 100644
--- a/src/main/cpp/aprinitializer.cpp
+++ b/src/main/cpp/aprinitializer.cpp
@@ -46,6 +46,7 @@ APRInitializer::APRInitializer() : p(0), startTime(0), tlsKey(0)
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() : p(0), startTime(0), tlsKey(0)
APRInitializer::~APRInitializer()
{
- {
+ stopWatchDogs();
+ isDestructed = true;
#if APR_HAS_THREADS
- std::unique_lock<std::mutex> lock(mutex);
- apr_threadkey_private_delete(tlsKey);
+ std::unique_lock<std::mutex> lock(mutex);
+ apr_threadkey_private_delete(tlsKey);
+#else
+ apr_terminate();
#endif
+}
- for (std::list<FileWatchdog*>::iterator iter = watchdogs.begin();
- iter != watchdogs.end();
- iter++)
- {
- delete *iter;
- }
+void APRInitializer::stopWatchDogs()
+{
+#if APR_HAS_THREADS
+ std::unique_lock<std::mutex> lock(mutex);
+#endif
+
+ while (!watchdogs.empty())
+ {
+ delete watchdogs.back();
+ watchdogs.pop_back();
}
+}
- // TODO LOGCXX-322
-#ifndef APR_HAS_THREADS
- apr_terminate();
-#endif
- isDestructed = true;
+void APRInitializer::unregisterAll()
+{
+ getInstance().stopWatchDogs();
}
APRInitializer& APRInitializer::getInstance()
@@ -132,7 +140,7 @@ void APRInitializer::addObject(size_t key, const ObjectPtr& pObject)
#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 @@ const ObjectPtr& APRInitializer::findOrAddObject(size_t key, std::function<Objec
#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 4b642981..97a594bf 100644
--- a/src/main/cpp/defaultconfigurator.cpp
+++ b/src/main/cpp/defaultconfigurator.cpp
@@ -67,7 +67,8 @@ void DefaultConfigurator::configure(LoggerRepositoryPtr repository)
OptionConverter::selectAndConfigure(
configuration,
configuratorClassName,
- repo);
+ repo,
+ getConfigurationWatchDelay());
}
else
{
@@ -113,5 +114,16 @@ const LogString DefaultConfigurator::getConfigurationFileName()
}
+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 264c1eb0..7f74de31 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::checkAndConfigure()
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 @@ void FileWatchdog::run()
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 ca21d063..bbd03870 100644
--- a/src/main/cpp/logmanager.cpp
+++ b/src/main/cpp/logmanager.cpp
@@ -200,6 +200,7 @@ LoggerList LogManager::getCurrentLoggers()
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 58605d84..9a4b4bc2 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 @@ ObjectPtr OptionConverter::instantiateByClassName(const LogString& className,
}
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 @@ void OptionConverter::selectAndConfigure(const File& configFileName,
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 77cf6b06..d5d3f554 100644
--- a/src/main/include/log4cxx/defaultconfigurator.h
+++ b/src/main/include/log4cxx/defaultconfigurator.h
@@ -47,6 +47,7 @@ class LOG4CXX_EXPORT DefaultConfigurator
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 c6edab08..4b2e5019 100644
--- a/src/main/include/log4cxx/helpers/aprinitializer.h
+++ b/src/main/include/log4cxx/helpers/aprinitializer.h
@@ -55,6 +55,7 @@ class APRInitializer
*/
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 @@ class APRInitializer
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 f3304fdb..4b20ac65 100644
--- a/src/main/include/log4cxx/helpers/optionconverter.h
+++ b/src/main/include/log4cxx/helpers/optionconverter.h
@@ -153,9 +153,12 @@ class LOG4CXX_EXPORT OptionConverter
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 a7758d31..a5e4e915 100644
--- a/src/test/cpp/CMakeLists.txt
+++ b/src/test/cpp/CMakeLists.txt
@@ -124,7 +124,7 @@ foreach(testName IN LISTS ALL_LOG4CXX_TESTS)
)
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 4953e2e0..99546660 100644
--- a/src/test/cpp/autoconfiguretestcase.cpp
+++ b/src/test/cpp/autoconfiguretestcase.cpp
@@ -47,6 +47,7 @@ LOGUNIT_CLASS(AutoConfigureTestCase)
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 @@ public:
LOGUNIT_ASSERT(debugLogger);
LOGUNIT_ASSERT(debugLogger->isDebugEnabled());
}
+
+ void stop()
+ {
+ LogManager::shutdown();
+ }
};
LOGUNIT_TEST_SUITE_REGISTRATION(AutoConfigureTestCase);