This is an automated email from the ASF dual-hosted git repository. swebb2066 pushed a commit to branch fix_watchdog_raw_pointers in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit a873b43e3aad16d85a11bc13f4c68d1e531e1433 Author: Stephen Webb <[email protected]> AuthorDate: Tue Jul 1 09:54:06 2025 +1000 Prevent undefined behaviour when reconfiguring after LogManager::shutdown --- src/main/cpp/aprinitializer.cpp | 27 ++------- src/main/cpp/domconfigurator.cpp | 68 ++++++---------------- src/main/cpp/filewatchdog.cpp | 5 ++ src/main/cpp/logmanager.cpp | 1 - src/main/cpp/propertyconfigurator.cpp | 29 ++++----- src/main/include/log4cxx/helpers/aprinitializer.h | 3 +- src/main/include/log4cxx/helpers/filewatchdog.h | 5 ++ src/main/include/log4cxx/helpers/singletonholder.h | 5 ++ src/main/include/log4cxx/propertyconfigurator.h | 5 +- src/main/include/log4cxx/xml/domconfigurator.h | 1 - 10 files changed, 56 insertions(+), 93 deletions(-) diff --git a/src/main/cpp/aprinitializer.cpp b/src/main/cpp/aprinitializer.cpp index a4346e72..0e7e2ea1 100644 --- a/src/main/cpp/aprinitializer.cpp +++ b/src/main/cpp/aprinitializer.cpp @@ -104,7 +104,6 @@ APRInitializer::APRInitializer() : APRInitializer::~APRInitializer() { - stopWatchDogs(); isDestructed = true; #if APR_HAS_THREADS std::lock_guard<std::mutex> lock(m_priv->mutex); @@ -112,22 +111,11 @@ APRInitializer::~APRInitializer() #endif } -void APRInitializer::stopWatchDogs() -{ - std::lock_guard<std::mutex> lock(m_priv->mutex); - - while (!m_priv->watchdogs.empty()) - { - m_priv->watchdogs.back()->stop(); - delete m_priv->watchdogs.back(); - m_priv->watchdogs.pop_back(); - } -} - +#if LOG4CXX_ABI_VERSION <= 15 void APRInitializer::unregisterAll() { - getInstance().stopWatchDogs(); } +#endif APRInitializer& APRInitializer::getInstance() { @@ -159,22 +147,15 @@ apr_threadkey_t* APRInitializer::getTlsKey() return getInstance().m_priv->tlsKey; } +#if LOG4CXX_ABI_VERSION <= 15 void APRInitializer::registerCleanup(FileWatchdog* watchdog) { - APRInitializer& instance(getInstance()); - std::lock_guard<std::mutex> lock(instance.m_priv->mutex); - instance.m_priv->watchdogs.push_back(watchdog); } void APRInitializer::unregisterCleanup(FileWatchdog* watchdog) { - APRInitializer& instance(getInstance()); - std::lock_guard<std::mutex> lock(instance.m_priv->mutex); - - auto iter = std::find(instance.m_priv->watchdogs.begin(), instance.m_priv->watchdogs.end(), watchdog); - if (iter != instance.m_priv->watchdogs.end()) - instance.m_priv->watchdogs.erase(iter); } +#endif void APRInitializer::addObject(size_t key, const ObjectPtr& pObject) { diff --git a/src/main/cpp/domconfigurator.cpp b/src/main/cpp/domconfigurator.cpp index b8d5cc8f..ac448574 100644 --- a/src/main/cpp/domconfigurator.cpp +++ b/src/main/cpp/domconfigurator.cpp @@ -49,6 +49,7 @@ #include <log4cxx/net/smtpappender.h> #include <log4cxx/helpers/messagebuffer.h> #include <log4cxx/helpers/threadutility.h> +#include <log4cxx/helpers/singletonholder.h> #define LOG4CXX 1 #include <log4cxx/helpers/aprinitializer.h> @@ -89,11 +90,22 @@ class XMLWatchdog : public FileWatchdog DOMConfigurator().doConfigure(file(), LogManager::getLoggerRepository()); } + + static void startWatching(const File& filename, long delay) + { + using WatchdogHolder = SingletonHolder<XMLWatchdog>; + auto pHolder = APRInitializer::getOrAddUnique<WatchdogHolder> + ( [&filename]() -> ObjectPtr + { return std::make_shared<WatchdogHolder>(filename); } + ); + auto& xdog = pHolder->value(); + xdog.setFile(filename); + xdog.setDelay(0 < delay ? delay : FileWatchdog::DEFAULT_DELAY); + xdog.start(); + } }; } } -XMLWatchdog* DOMConfigurator::xdog = NULL; - IMPLEMENT_LOG4CXX_OBJECT(DOMConfigurator) @@ -978,19 +990,8 @@ spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const std::string& f spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const File& file, long delay) { - if ( xdog ) - { - APRInitializer::unregisterCleanup(xdog); - delete xdog; - } - spi::ConfigurationStatus status = DOMConfigurator().doConfigure(file, LogManager::getLoggerRepository()); - - xdog = new XMLWatchdog(file); - APRInitializer::registerCleanup(xdog); - xdog->setDelay(0 < delay ? delay : FileWatchdog::DEFAULT_DELAY); - xdog->start(); - + XMLWatchdog::startWatching(file, delay); return status; } @@ -999,19 +1000,8 @@ spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const File& file, lo spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const std::wstring& filename, long delay) { File file(filename); - if ( xdog ) - { - APRInitializer::unregisterCleanup(xdog); - delete xdog; - } - spi::ConfigurationStatus status = DOMConfigurator().doConfigure(file, LogManager::getLoggerRepository()); - - xdog = new XMLWatchdog(file); - APRInitializer::registerCleanup(xdog); - xdog->setDelay(delay); - xdog->start(); - + XMLWatchdog::startWatching(file, delay); return status; } #endif @@ -1020,19 +1010,8 @@ spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const std::wstring& spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const std::basic_string<UniChar>& filename, long delay) { File file(filename); - if ( xdog ) - { - APRInitializer::unregisterCleanup(xdog); - delete xdog; - } - spi::ConfigurationStatus status = DOMConfigurator().doConfigure(file, LogManager::getLoggerRepository()); - - xdog = new XMLWatchdog(file); - APRInitializer::registerCleanup(xdog); - xdog->setDelay(delay); - xdog->start(); - + XMLWatchdog::startWatching(file, delay); return status; } #endif @@ -1041,19 +1020,8 @@ spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const std::basic_str spi::ConfigurationStatus DOMConfigurator::configureAndWatch(const CFStringRef& filename, long delay) { File file(filename); - if ( xdog ) - { - APRInitializer::unregisterCleanup(xdog); - delete xdog; - } - spi::ConfigurationStatus status = DOMConfigurator().doConfigure(file, LogManager::getLoggerRepository()); - - xdog = new XMLWatchdog(file); - APRInitializer::registerCleanup(xdog); - xdog->setDelay(delay); - xdog->start(); - + XMLWatchdog::startWatching(file, delay); return status; } #endif diff --git a/src/main/cpp/filewatchdog.cpp b/src/main/cpp/filewatchdog.cpp index 2cd4c49f..73ffc732 100644 --- a/src/main/cpp/filewatchdog.cpp +++ b/src/main/cpp/filewatchdog.cpp @@ -163,3 +163,8 @@ void FileWatchdog::setDelay(long delay1){ ); } } + +void FileWatchdog::setFile(const File& filename) +{ + m_priv->file = filename; +} diff --git a/src/main/cpp/logmanager.cpp b/src/main/cpp/logmanager.cpp index 36fedd00..935154de 100644 --- a/src/main/cpp/logmanager.cpp +++ b/src/main/cpp/logmanager.cpp @@ -203,7 +203,6 @@ LoggerList LogManager::getCurrentLoggers() void LogManager::shutdown() { - APRInitializer::unregisterAll(); ThreadUtility::instance()->removeAllPeriodicTasks(); getLoggerRepository()->shutdown(); } diff --git a/src/main/cpp/propertyconfigurator.cpp b/src/main/cpp/propertyconfigurator.cpp index 922edac5..9586a13b 100644 --- a/src/main/cpp/propertyconfigurator.cpp +++ b/src/main/cpp/propertyconfigurator.cpp @@ -36,6 +36,7 @@ #include <log4cxx/helpers/fileinputstream.h> #include <log4cxx/helpers/loader.h> #include <log4cxx/helpers/threadutility.h> +#include <log4cxx/helpers/singletonholder.h> #include <log4cxx/rolling/rollingfileappender.h> #define LOG4CXX 1 @@ -67,11 +68,22 @@ class PropertyWatchdog : public FileWatchdog PropertyConfigurator().doConfigure(file(), LogManager::getLoggerRepository()); } + + static void startWatching(const File& filename, long delay) + { + using WatchdogHolder = SingletonHolder<PropertyWatchdog>; + auto pHolder = APRInitializer::getOrAddUnique<WatchdogHolder> + ( [&filename]() -> ObjectPtr + { return std::make_shared<WatchdogHolder>(filename); } + ); + auto& pdog = pHolder->value(); + pdog.setFile(filename); + pdog.setDelay(0 < delay ? delay : FileWatchdog::DEFAULT_DELAY); + pdog.start(); + } }; } -PropertyWatchdog* PropertyConfigurator::pdog = NULL; - IMPLEMENT_LOG4CXX_OBJECT(PropertyConfigurator) PropertyConfigurator::PropertyConfigurator() @@ -155,19 +167,8 @@ spi::ConfigurationStatus PropertyConfigurator::configureAndWatch(const File& con spi::ConfigurationStatus PropertyConfigurator::configureAndWatch( const File& configFilename, long delay) { - if (pdog) - { - APRInitializer::unregisterCleanup(pdog); - delete pdog; - } - spi::ConfigurationStatus stat = PropertyConfigurator().doConfigure(configFilename, LogManager::getLoggerRepository()); - - pdog = new PropertyWatchdog(configFilename); - APRInitializer::registerCleanup(pdog); - pdog->setDelay(0 < delay ? delay : FileWatchdog::DEFAULT_DELAY); - pdog->start(); - + PropertyWatchdog::startWatching(configFilename, delay); return stat; } diff --git a/src/main/include/log4cxx/helpers/aprinitializer.h b/src/main/include/log4cxx/helpers/aprinitializer.h index f810127a..aab18595 100644 --- a/src/main/include/log4cxx/helpers/aprinitializer.h +++ b/src/main/include/log4cxx/helpers/aprinitializer.h @@ -52,6 +52,7 @@ class APRInitializer static apr_threadkey_t* getTlsKey(); static bool isDestructed; +#if LOG4CXX_ABI_VERSION <= 15 /** * Register a FileWatchdog for deletion prior to termination. * FileWatchdog must be @@ -60,6 +61,7 @@ class APRInitializer static void registerCleanup(FileWatchdog* watchdog); static void unregisterCleanup(FileWatchdog* watchdog); static void unregisterAll(); +#endif /** * Store a single instance type ObjectPtr for deletion prior to termination */ @@ -84,7 +86,6 @@ 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 LOG4CXX_DECLARE_PRIVATE_MEMBER_PTR(APRInitializerPrivate, m_priv) private: // Class methods diff --git a/src/main/include/log4cxx/helpers/filewatchdog.h b/src/main/include/log4cxx/helpers/filewatchdog.h index 2340fadc..710dfa2e 100644 --- a/src/main/include/log4cxx/helpers/filewatchdog.h +++ b/src/main/include/log4cxx/helpers/filewatchdog.h @@ -53,6 +53,11 @@ class LOG4CXX_EXPORT FileWatchdog */ void setDelay(long delay); + /** + Change the watched file to \c filename. + */ + void setFile(const File& filename); + /** Create an asynchronous task that periodically checks for a file change after first calling doOnChange(). */ diff --git a/src/main/include/log4cxx/helpers/singletonholder.h b/src/main/include/log4cxx/helpers/singletonholder.h index 85783e0d..b52c0766 100644 --- a/src/main/include/log4cxx/helpers/singletonholder.h +++ b/src/main/include/log4cxx/helpers/singletonholder.h @@ -42,6 +42,11 @@ public: // Object method stubs LOG4CXX_CAST_ENTRY(ThisType) END_LOG4CXX_CAST_MAP() +public: // ...structors + SingletonHolder() {} + template <class ParamType> + SingletonHolder(const ParamType& arg) : m_data(arg) {} + public: // Accessors T& value() { return m_data; } }; diff --git a/src/main/include/log4cxx/propertyconfigurator.h b/src/main/include/log4cxx/propertyconfigurator.h index 6f85d2b8..b5e5872b 100644 --- a/src/main/include/log4cxx/propertyconfigurator.h +++ b/src/main/include/log4cxx/propertyconfigurator.h @@ -64,9 +64,9 @@ support for {@link spi::Filter Filters}, custom {@link spi::ErrorHandler ErrorHandlers}, nested appenders such as the {@link AsyncAppender AsyncAppender}, etc. -<h3>Appender configuration</h3> +<h3>Configuring appenders</h3> -Appender configuration syntax is: +%Appender configuration syntax is: <pre> # For appender named <i>appenderName</i>, set its class. # Note: The appender name can contain dots. @@ -391,7 +391,6 @@ class LOG4CXX_EXPORT PropertyConfigurator : private: PropertyConfigurator(const PropertyConfigurator&); PropertyConfigurator& operator=(const PropertyConfigurator&); - static PropertyWatchdog* pdog; }; // class PropertyConfigurator } // namespace log4cxx diff --git a/src/main/include/log4cxx/xml/domconfigurator.h b/src/main/include/log4cxx/xml/domconfigurator.h index beea5953..16ae64b7 100644 --- a/src/main/include/log4cxx/xml/domconfigurator.h +++ b/src/main/include/log4cxx/xml/domconfigurator.h @@ -374,7 +374,6 @@ class LOG4CXX_EXPORT DOMConfigurator : // prevent assignment or copy statements DOMConfigurator(const DOMConfigurator&); DOMConfigurator& operator=(const DOMConfigurator&); - static XMLWatchdog* xdog; LOG4CXX_DECLARE_PRIVATE_MEMBER_PTR(DOMConfiguratorPrivate, m_priv) };
