This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
The following commit(s) were added to refs/heads/master by this push:
new 86ea0a77 Prevent undefined behaviour when reconfiguring after
LogManager::shutdown (#504)
86ea0a77 is described below
commit 86ea0a77c8dfe6466358536db2755f3473347e8a
Author: Stephen Webb <[email protected]>
AuthorDate: Wed Jul 2 09:37:32 2025 +1000
Prevent undefined behaviour when reconfiguring after LogManager::shutdown
(#504)
---
.github/workflows/log4cxx-windows-static.yml | 6 +-
.github/workflows/log4cxx-windows.yml | 6 +-
src/main/cpp/aprinitializer.cpp | 28 ++-------
src/main/cpp/domconfigurator.cpp | 68 ++++++----------------
src/main/cpp/filewatchdog.cpp | 5 ++
src/main/cpp/logmanager.cpp | 1 -
src/main/cpp/optionconverter.cpp | 22 +++++--
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 | 7 +++
src/main/include/log4cxx/propertyconfigurator.h | 5 +-
src/main/include/log4cxx/xml/domconfigurator.h | 1 -
src/site/markdown/change-report-gh.md | 4 ++
14 files changed, 84 insertions(+), 106 deletions(-)
diff --git a/.github/workflows/log4cxx-windows-static.yml
b/.github/workflows/log4cxx-windows-static.yml
index 4712728c..04a08f33 100644
--- a/.github/workflows/log4cxx-windows-static.yml
+++ b/.github/workflows/log4cxx-windows-static.yml
@@ -24,12 +24,12 @@ jobs:
strategy:
fail-fast: false
matrix:
- name: [windows-2019, windows-2022]
+ name: [windows-2022, windows-2025]
include:
- - name: windows-2019
- os: windows-2019
- name: windows-2022
os: windows-2022
+ - name: windows-2025
+ os: windows-2025
steps:
- uses: actions/checkout@v4
diff --git a/.github/workflows/log4cxx-windows.yml
b/.github/workflows/log4cxx-windows.yml
index 69ae88bd..393d2cd4 100644
--- a/.github/workflows/log4cxx-windows.yml
+++ b/.github/workflows/log4cxx-windows.yml
@@ -24,12 +24,12 @@ jobs:
strategy:
fail-fast: false
matrix:
- name: [windows-2019, windows-2022]
+ name: [windows-2022, windows-2025]
include:
- - name: windows-2019
- os: windows-2019
- name: windows-2022
os: windows-2022
+ - name: windows-2025
+ os: windows-2025
steps:
- uses: actions/checkout@v4
diff --git a/src/main/cpp/aprinitializer.cpp b/src/main/cpp/aprinitializer.cpp
index a4346e72..8e62af21 100644
--- a/src/main/cpp/aprinitializer.cpp
+++ b/src/main/cpp/aprinitializer.cpp
@@ -52,7 +52,6 @@ struct APRInitializer::APRInitializerPrivate{
apr_pool_t* p;
std::mutex mutex;
- std::list<FileWatchdog*> watchdogs;
log4cxx_time_t startTime;
apr_threadkey_t* tlsKey;
std::vector<IdentifiedObject> objects;
@@ -104,7 +103,6 @@ APRInitializer::APRInitializer() :
APRInitializer::~APRInitializer()
{
- stopWatchDogs();
isDestructed = true;
#if APR_HAS_THREADS
std::lock_guard<std::mutex> lock(m_priv->mutex);
@@ -112,22 +110,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 +146,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/optionconverter.cpp b/src/main/cpp/optionconverter.cpp
index 6fa7b502..f70baef3 100644
--- a/src/main/cpp/optionconverter.cpp
+++ b/src/main/cpp/optionconverter.cpp
@@ -42,6 +42,7 @@
#endif
#include <log4cxx/helpers/aprinitializer.h>
#include <log4cxx/helpers/filewatchdog.h>
+#include <log4cxx/helpers/singletonholder.h>
namespace LOG4CXX_NS
{
@@ -65,6 +66,20 @@ class ConfiguratorWatchdog : public helpers::FileWatchdog
{
m_config->doConfigure(file(), LogManager::getLoggerRepository());
}
+
+ static void startWatching(const spi::ConfiguratorPtr& config, const
File& filename, long delay)
+ {
+ using WatchdogHolder =
helpers::SingletonHolder<ConfiguratorWatchdog>;
+ auto pHolder =
helpers::APRInitializer::getOrAddUnique<WatchdogHolder>
+ ( [&config, &filename]() -> helpers::ObjectPtr
+ { return
std::make_shared<WatchdogHolder>(config, filename); }
+ );
+ auto& dog = pHolder->value();
+ dog.m_config = config;
+ dog.setFile(filename);
+ dog.setDelay(delay);
+ dog.start();
+ }
};
}
@@ -465,12 +480,7 @@ void OptionConverter::selectAndConfigure(const File&
configFileName,
}
if (0 < delay)
- {
- auto dog = new ConfiguratorWatchdog(configurator,
configFileName);
- APRInitializer::registerCleanup(dog);
- dog->setDelay(delay);
- dog->start();
- }
+ ConfiguratorWatchdog::startWatching(configurator,
configFileName, delay);
else
configurator->doConfigure(configFileName, hierarchy);
}
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..66651d99 100644
--- a/src/main/include/log4cxx/helpers/singletonholder.h
+++ b/src/main/include/log4cxx/helpers/singletonholder.h
@@ -42,6 +42,13 @@ public: // Object method stubs
LOG4CXX_CAST_ENTRY(ThisType)
END_LOG4CXX_CAST_MAP()
+public: // ...structors
+ SingletonHolder() {}
+ template <typename ... Args>
+ SingletonHolder(Args&& ... args)
+ : m_data(std::forward<Args>(args) ... )
+ {}
+
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)
};
diff --git a/src/site/markdown/change-report-gh.md
b/src/site/markdown/change-report-gh.md
index 396e21e1..5a64f442 100644
--- a/src/site/markdown/change-report-gh.md
+++ b/src/site/markdown/change-report-gh.md
@@ -60,6 +60,10 @@ Release 1.5.0 includes the following new features:
The following issues have been addressed:
+* Undefined behaviour when reloading a configuration file after calling
LogManager::shutdown
+ \[[#504](https://github.com/apache/logging-log4cxx/issues/504)\]
+* Compilation error when using gcc on MacOS
+ \[[#499](https://github.com/apache/logging-log4cxx/issues/499)\]
* It was possible for logging events to be lost when reloading a configuration
file
\[[#491](https://github.com/apache/logging-log4cxx/issues/491)\]
