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 2025/03/26 01:48:20 UTC
(logging-log4cxx) branch reconfiguration_synchronization created (now 101cd36f)
This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a change to branch reconfiguration_synchronization
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
at 101cd36f Ensure log events are sent to an appender during a reconfiguration event
This branch includes the following new commits:
new 101cd36f Ensure log events are sent to an appender during a reconfiguration event
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(logging-log4cxx) 01/01: Ensure log events are sent to an appender during a reconfiguration event
Posted by sw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch reconfiguration_synchronization
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit 101cd36fd4638e7a2e99ed77737cad47b222da2e
Author: Stephen Webb <sw...@gmail.com>
AuthorDate: Tue Mar 25 11:49:28 2025 +1100
Ensure log events are sent to an appender during a reconfiguration event
---
src/main/cpp/appenderattachableimpl.cpp | 33 ++++++++++++++++++++++
src/main/cpp/asyncappender.cpp | 10 +++++++
src/main/cpp/domconfigurator.cpp | 20 ++++++-------
src/main/cpp/fallbackerrorhandler.cpp | 18 +++++-------
src/main/cpp/logger.cpp | 30 ++++++++++++++------
src/main/include/log4cxx/asyncappender.h | 11 ++++++++
.../log4cxx/helpers/appenderattachableimpl.h | 11 ++++++++
src/main/include/log4cxx/logger.h | 23 +++++++++++----
src/main/include/log4cxx/spi/appenderattachable.h | 18 ++++++++++++
src/test/cpp/varia/errorhandlertestcase.cpp | 27 ++++++++++++++++--
src/test/resources/input/xml/fallback1.xml | 12 +-------
src/test/resources/input/xml/fallback2.xml | 11 +-------
12 files changed, 164 insertions(+), 60 deletions(-)
diff --git a/src/main/cpp/appenderattachableimpl.cpp b/src/main/cpp/appenderattachableimpl.cpp
index ddd4cc3c..1f9e32a6 100644
--- a/src/main/cpp/appenderattachableimpl.cpp
+++ b/src/main/cpp/appenderattachableimpl.cpp
@@ -167,4 +167,37 @@ void AppenderAttachableImpl::removeAppender(const LogString& name)
}
}
+bool AppenderAttachableImpl::replaceAppender(const AppenderPtr& oldAppender, const AppenderPtr& newAppender)
+{
+ bool found = false;
+ if (m_priv && oldAppender && newAppender)
+ {
+ auto oldName = oldAppender->getName();
+ std::lock_guard<std::mutex> lock( m_priv->m_mutex );
+ auto it = std::find_if(m_priv->appenderList.begin(), m_priv->appenderList.end()
+ , [&oldName](const AppenderPtr& appender) -> bool
+ {
+ return oldName == appender->getName();
+ });
+ if (it != m_priv->appenderList.end())
+ {
+ m_priv->appenderList.erase(it);
+ m_priv->appenderList.push_back(newAppender);
+ found = true;
+ }
+ }
+ return found;
+}
+
+void AppenderAttachableImpl::replaceAppenders(const AppenderList& newList)
+{
+ auto oldAppenders = getAllAppenders();
+ if (!m_priv)
+ m_priv = std::make_unique<AppenderAttachableImpl::priv_data>();
+ std::lock_guard<std::mutex> lock( m_priv->m_mutex );
+ for (auto a : oldAppenders)
+ a->close();
+ m_priv->appenderList = newList;
+}
+
diff --git a/src/main/cpp/asyncappender.cpp b/src/main/cpp/asyncappender.cpp
index 7c7caa8d..cec9f359 100644
--- a/src/main/cpp/asyncappender.cpp
+++ b/src/main/cpp/asyncappender.cpp
@@ -413,6 +413,16 @@ void AsyncAppender::removeAppender(const LogString& n)
priv->appenders.removeAppender(n);
}
+bool AsyncAppender::replaceAppender(const AppenderPtr& oldAppender, const AppenderPtr& newAppender)
+{
+ return priv->appenders.replaceAppender(oldAppender, newAppender);
+}
+
+void AsyncAppender::replaceAppenders( const AppenderList& newList)
+{
+ priv->appenders.replaceAppenders(newList);
+}
+
bool AsyncAppender::getLocationInfo() const
{
return priv->locationInfo;
diff --git a/src/main/cpp/domconfigurator.cpp b/src/main/cpp/domconfigurator.cpp
index 12648642..b8d5cc8f 100644
--- a/src/main/cpp/domconfigurator.cpp
+++ b/src/main/cpp/domconfigurator.cpp
@@ -535,10 +535,6 @@ void DOMConfigurator::parseChildrenOfLoggerElement(
PropertySetter propSetter(logger);
std::vector<AppenderPtr> newappenders;
- // Remove all existing appenders from logger. They will be
- // reconstructed if need be.
- logger->removeAllAppenders();
-
for (apr_xml_elem* currentElement = loggerElement->first_child;
currentElement;
currentElement = currentElement->next)
@@ -550,12 +546,12 @@ void DOMConfigurator::parseChildrenOfLoggerElement(
AppenderPtr appender = findAppenderByReference(p, utf8Decoder, currentElement, doc, appenders);
LogString refName = subst(getAttribute(utf8Decoder, currentElement, REF_ATTR));
- if (!LogLog::isDebugEnabled())
- ;
- else if (appender != 0)
+ if (appender)
{
- LogLog::debug(LOG4CXX_STR("Adding appender named [") + refName +
- LOG4CXX_STR("] to logger [") + logger->getName() + LOG4CXX_STR("]."));
+ if (LogLog::isDebugEnabled())
+ LogLog::debug(LOG4CXX_STR("Adding appender named [") + refName +
+ LOG4CXX_STR("] to logger [") + logger->getName() + LOG4CXX_STR("]."));
+ newappenders.push_back(appender);
}
else
{
@@ -563,8 +559,6 @@ void DOMConfigurator::parseChildrenOfLoggerElement(
LOG4CXX_STR("] not found."));
}
- logger->addAppender(appender);
-
}
else if (tagName == LEVEL_TAG)
{
@@ -579,6 +573,10 @@ void DOMConfigurator::parseChildrenOfLoggerElement(
setParameter(p, utf8Decoder, currentElement, propSetter);
}
}
+ if (newappenders.empty())
+ logger->removeAllAppenders();
+ else
+ logger->replaceAppenders(newappenders);
propSetter.activate(p);
}
diff --git a/src/main/cpp/fallbackerrorhandler.cpp b/src/main/cpp/fallbackerrorhandler.cpp
index 0b5f7d93..bed0530e 100644
--- a/src/main/cpp/fallbackerrorhandler.cpp
+++ b/src/main/cpp/fallbackerrorhandler.cpp
@@ -87,22 +87,18 @@ void FallbackErrorHandler::error(const LogString& message,
{
if (LogLog::isDebugEnabled())
{
- LogLog::debug(((LogString) LOG4CXX_STR("FB: Searching for ["))
- + primaryLocked->getName() + LOG4CXX_STR("] in logger [")
- + l->getName() + LOG4CXX_STR("]."));
- LogLog::debug(((LogString) LOG4CXX_STR("FB: Replacing ["))
- + primaryLocked->getName() + LOG4CXX_STR("] by [")
+ LogLog::debug(LOG4CXX_STR("FB: Replacing [")
+ + primaryLocked->getName() + LOG4CXX_STR("] with [")
+ backupLocked->getName() + LOG4CXX_STR("] in logger [")
+ l->getName() + LOG4CXX_STR("]."));
}
- l->removeAppender(primaryLocked);
- if (LogLog::isDebugEnabled())
+ if (!l->replaceAppender(primaryLocked, backupLocked))
{
- LogLog::debug(((LogString) LOG4CXX_STR("FB: Adding appender ["))
- + backupLocked->getName() + LOG4CXX_STR("] to logger ")
- + l->getName());
+ LogLog::warn(LOG4CXX_STR("FB: Failed to replace [")
+ + primaryLocked->getName() + LOG4CXX_STR("] with [")
+ + backupLocked->getName() + LOG4CXX_STR("] in logger [")
+ + l->getName() + LOG4CXX_STR("]."));
}
- l->addAppender(backupLocked);
}
m_priv->errorReported = true;
}
diff --git a/src/main/cpp/logger.cpp b/src/main/cpp/logger.cpp
index 75781453..e814620f 100644
--- a/src/main/cpp/logger.cpp
+++ b/src/main/cpp/logger.cpp
@@ -112,23 +112,35 @@ void Logger::addAppender(const AppenderPtr newAppender)
}
}
-void Logger::reconfigure( const std::vector<AppenderPtr>& appenders, bool additive1 )
+bool Logger::replaceAppender(const AppenderPtr& oldAppender, const AppenderPtr& newAppender)
{
- m_priv->additive = additive1;
+ bool result = m_priv->aai.replaceAppender(oldAppender, newAppender);
+ if (result)
+ {
+ if (auto rep = getHierarchy())
+ rep->fireAddAppenderEvent(this, newAppender.get());
+ }
+ return result;
+}
- m_priv->aai.removeAllAppenders();
+void Logger::replaceAppenders( const AppenderList& newList)
+{
+ m_priv->aai.replaceAppenders(newList);
- for (auto const& item : appenders)
+ if (auto rep = getHierarchy())
{
- m_priv->aai.addAppender(item);
-
- if (auto rep = getHierarchy())
- {
+ for (auto const& item : newList)
rep->fireAddAppenderEvent(this, item.get());
- }
}
}
+void Logger::reconfigure( const AppenderList& newList, bool newAdditivity )
+{
+ m_priv->additive = newAdditivity;
+
+ replaceAppenders(newList);
+}
+
void Logger::callAppenders(const spi::LoggingEventPtr& event, Pool& p) const
{
int writes = 0;
diff --git a/src/main/include/log4cxx/asyncappender.h b/src/main/include/log4cxx/asyncappender.h
index b957d03a..e1ea6c8e 100644
--- a/src/main/include/log4cxx/asyncappender.h
+++ b/src/main/include/log4cxx/asyncappender.h
@@ -152,6 +152,17 @@ class LOG4CXX_EXPORT AsyncAppender :
*/
void removeAppender(const LogString& name) override;
+ /**
+ * Replace \c oldAppender with \c newAppender.
+ * @return true if oldAppender was replaced with newAppender.
+ */
+ bool replaceAppender(const AppenderPtr& oldAppender, const AppenderPtr& newAppender) LOG4CXX_16_VIRTUAL_SPECIFIER;
+
+ /**
+ * Replace any previously added appenders with \c newList.
+ */
+ void replaceAppenders(const AppenderList& newList) LOG4CXX_16_VIRTUAL_SPECIFIER;
+
/**
* The <b>LocationInfo</b> attribute is provided for compatibility
* with log4j and has no effect on the log output.
diff --git a/src/main/include/log4cxx/helpers/appenderattachableimpl.h b/src/main/include/log4cxx/helpers/appenderattachableimpl.h
index 73c370ec..289512e9 100644
--- a/src/main/include/log4cxx/helpers/appenderattachableimpl.h
+++ b/src/main/include/log4cxx/helpers/appenderattachableimpl.h
@@ -101,6 +101,17 @@ class LOG4CXX_EXPORT AppenderAttachableImpl :
*/
void removeAppender(const LogString& name) override;
+ /**
+ * Replace \c oldAppender with \c newAppender.
+ * @return true if oldAppender was replaced with newAppender.
+ */
+ bool replaceAppender(const AppenderPtr& oldAppender, const AppenderPtr& newAppender) LOG4CXX_16_VIRTUAL_SPECIFIER;
+
+ /**
+ * Replace any previously added appenders with \c newList.
+ */
+ void replaceAppenders(const AppenderList& newList) LOG4CXX_16_VIRTUAL_SPECIFIER;
+
private:
LOG4CXX_DECLARE_PRIVATE_MEMBER_PTR(priv_data, m_priv)
diff --git a/src/main/include/log4cxx/logger.h b/src/main/include/log4cxx/logger.h
index f16aaf58..91181504 100644
--- a/src/main/include/log4cxx/logger.h
+++ b/src/main/include/log4cxx/logger.h
@@ -788,8 +788,7 @@ class LOG4CXX_EXPORT Logger
Return the the LoggerRepository where this
<code>Logger</code> is attached.
*/
- LOG4CXX_NS::spi::LoggerRepository* getLoggerRepository() const;
-
+ spi::LoggerRepository* getLoggerRepository() const;
/**
* Get the logger name.
@@ -1791,6 +1790,17 @@ class LOG4CXX_EXPORT Logger
*/
void removeAppender(const LogString& name) override;
+ /**
+ * Replace \c oldAppender with \c newAppender.
+ * @return true if oldAppender was replaced with newAppender.
+ */
+ bool replaceAppender(const AppenderPtr& oldAppender, const AppenderPtr& newAppender) LOG4CXX_16_VIRTUAL_SPECIFIER;
+
+ /**
+ * Replace all previously added appenders with \c newList.
+ */
+ void replaceAppenders(const AppenderList& newList) LOG4CXX_16_VIRTUAL_SPECIFIER;
+
/**
Set the additivity flag for this logger.
*/
@@ -2095,12 +2105,13 @@ class LOG4CXX_EXPORT Logger
void trace(const std::string& msg) const;
/**
- * Reconfigure this logger by configuring all of the appenders.
+ * Replace all current appenders with \c newList and
+ * set the additivity flag to \c newAdditivity.
*
- * @param appenders The appenders to set. Any currently existing appenders are removed.
- * @param additivity The additivity of this logger
+ * @param newList The appenders to set.
+ * @param newAdditivity Whether this logger should send events to its parent.
*/
- void reconfigure( const std::vector<AppenderPtr>& appenders, bool additivity );
+ void reconfigure( const AppenderList& newList, bool newAdditivity );
private:
//
diff --git a/src/main/include/log4cxx/spi/appenderattachable.h b/src/main/include/log4cxx/spi/appenderattachable.h
index 93f77605..2498dc96 100644
--- a/src/main/include/log4cxx/spi/appenderattachable.h
+++ b/src/main/include/log4cxx/spi/appenderattachable.h
@@ -72,6 +72,19 @@ class LOG4CXX_EXPORT AppenderAttachable : public virtual helpers::Object
*/
virtual void removeAppender(const LogString& name) = 0;
+#if 15 < LOG4CXX_ABI_VERSION
+ /**
+ * Replace \c oldAppender with \c newAppender.
+ * @return true if oldAppender was replaced with newAppender.
+ */
+ virtual bool replaceAppender(const AppenderPtr& oldAppender, const AppenderPtr& newAppender) = 0;
+
+ /**
+ * Replace any previously added appenders with \c newList.
+ */
+ virtual void replaceAppenders(const AppenderList& newList) = 0;
+#endif
+
// Dtor
virtual ~AppenderAttachable() {}
};
@@ -80,5 +93,10 @@ LOG4CXX_PTR_DEF(AppenderAttachable);
}
}
+#if 15 < LOG4CXX_ABI_VERSION
+#define LOG4CXX_16_VIRTUAL_SPECIFIER override
+#else
+#define LOG4CXX_16_VIRTUAL_SPECIFIER
+#endif
#endif //_LOG4CXX_SPI_APPENDER_ATTACHABLE_H_
diff --git a/src/test/cpp/varia/errorhandlertestcase.cpp b/src/test/cpp/varia/errorhandlertestcase.cpp
index c8cf517f..223cc10d 100644
--- a/src/test/cpp/varia/errorhandlertestcase.cpp
+++ b/src/test/cpp/varia/errorhandlertestcase.cpp
@@ -21,6 +21,7 @@
#include <log4cxx/varia/fallbackerrorhandler.h>
#include <log4cxx/appender.h>
#include <log4cxx/helpers/loglog.h>
+#include <log4cxx/helpers/pool.h>
#include "../logunit.h"
#include "../util/transformer.h"
#include "../util/compare.h"
@@ -28,6 +29,20 @@
#include "../util/linenumberfilter.h"
#include <iostream>
+namespace
+{
+ /*
+ The following path is carefully designed to fail on Linux and Windows,
+ so that the appender FALLBACK is used instead and the test succeeds in
+ the end. Linux considers "." as current directory, which can not be
+ created as a file, while Windows strips "/."[1] internally and fails at
+ ":", which is an invalid name.
+
+ [1]: https://en.wikipedia.org/wiki/Filename#Comparison_of_filename_limitations
+ */
+ log4cxx::LogString BadPath{ LOG4CXX_STR("output/xyz/:/.") };
+}
+
using namespace log4cxx;
using namespace log4cxx::helpers;
using namespace log4cxx::xml;
@@ -77,9 +92,12 @@ public:
log4cxx::spi::ErrorHandlerPtr errHandle = primary->getErrorHandler();
eh = log4cxx::cast<log4cxx::varia::FallbackErrorHandler>(errHandle);
LOGUNIT_ASSERT(eh != 0);
+ primary->setOption(LOG4CXX_STR("FILE"), BadPath);
+ Pool p;
+ primary->activateOptions(p);
+ LOGUNIT_ASSERT(eh->errorReported());
common();
- LOGUNIT_ASSERT(eh->errorReported());
std::string TEST1_PAT =
"FALLBACK - (root|test) - Message {0-9}";
@@ -117,9 +135,14 @@ public:
eh = log4cxx::cast<log4cxx::varia::FallbackErrorHandler>(errHandle);
LOGUNIT_ASSERT(eh != 0);
eh->setLogger(logger);
- common();
+
+ primary->setOption(LOG4CXX_STR("FILE"), BadPath);
+ Pool p;
+ primary->activateOptions(p);
LOGUNIT_ASSERT(eh->errorReported());
+ common();
+
std::string TEST1_PAT =
"FALLBACK - (root|test) - Message {0-9}";
diff --git a/src/test/resources/input/xml/fallback1.xml b/src/test/resources/input/xml/fallback1.xml
index 83ed810e..5b21911c 100644
--- a/src/test/resources/input/xml/fallback1.xml
+++ b/src/test/resources/input/xml/fallback1.xml
@@ -26,17 +26,7 @@
<root-ref/>
<appender-ref ref="FALLBACK" />
</errorHandler>
-
- <!--
- The following path is carefully designed to fail on Linux and Windows,
- so that the appender FALLBACK is used instead and the test succeeds in
- the end. Linux considers "." as current directory, which can not be
- created as a file, while Windows strips "/."[1] internally and fails at
- ":", which is an invalid name.
-
- [1]: https://en.wikipedia.org/wiki/Filename#Comparison_of_filename_limitations
- -->
- <param name="File" value="output/xyz/:/." />
+ <param name="File" value="output/xyz/x.log" />
<param name="Append" value="false" />
<layout class="org.apache.log4j.PatternLayout">
diff --git a/src/test/resources/input/xml/fallback2.xml b/src/test/resources/input/xml/fallback2.xml
index e0f37003..4fc6e5bc 100644
--- a/src/test/resources/input/xml/fallback2.xml
+++ b/src/test/resources/input/xml/fallback2.xml
@@ -27,16 +27,7 @@
<appender-ref ref="FALLBACK" />
</errorHandler>
- <!--
- The following path is carefully designed to fail on Linux and Windows,
- so that the appender FALLBACK is used instead and the test succeeds in
- the end. Linux considers "." as current directory, which can not be
- created as a file, while Windows strips "/."[1] internally and fails at
- ":", which is an invalid name.
-
- [1]: https://en.wikipedia.org/wiki/Filename#Comparison_of_filename_limitations
- -->
- <param name="File" value="output/xyz/:/." />
+ <param name="File" value="output/xyz/x.log" />
<param name="Append" value="false" />
<layout class="org.apache.log4j.PatternLayout">