This is an automated email from the ASF dual-hosted git repository. swebb2066 pushed a commit to branch serialize_threads_in_close in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit cdd2d9f8cc75453c31b1d4089f426b55271d43bd Author: Stephen Webb <[email protected]> AuthorDate: Fri Mar 6 15:32:17 2026 +1100 Prevent undefined behavior during a configuration change --- src/main/cpp/appenderskeleton.cpp | 8 ++++++++ src/main/cpp/dbappender.cpp | 4 +++- src/main/cpp/nteventlogappender.cpp | 2 ++ src/main/cpp/odbcappender.cpp | 2 +- src/main/cpp/smtpappender.cpp | 2 +- src/main/cpp/socketappenderskeleton.cpp | 2 +- src/main/cpp/syslogappender.cpp | 2 +- src/main/cpp/telnetappender.cpp | 10 ++++------ src/main/cpp/writerappender.cpp | 5 +---- src/main/include/log4cxx/private/appenderskeleton_priv.h | 5 +++++ src/test/cpp/vectorappender.cpp | 11 ++++++----- src/test/cpp/vectorappender.h | 6 +----- 12 files changed, 34 insertions(+), 25 deletions(-) diff --git a/src/main/cpp/appenderskeleton.cpp b/src/main/cpp/appenderskeleton.cpp index 7f63cd0f..ec7c44f7 100644 --- a/src/main/cpp/appenderskeleton.cpp +++ b/src/main/cpp/appenderskeleton.cpp @@ -144,6 +144,14 @@ bool AppenderSkeleton::AppenderSkeletonPrivate::checkNotClosed() return true; } +bool AppenderSkeleton::AppenderSkeletonPrivate::setClosed() +{ + std::lock_guard<std::recursive_mutex> lock(this->mutex); + bool wasOpen = !this->closed; + this->closed = true; + return wasOpen; +} + bool AppenderSkeleton::AppenderSkeletonPrivate::checkLayout() { if (!this->layout) diff --git a/src/main/cpp/dbappender.cpp b/src/main/cpp/dbappender.cpp index 1f38e2b1..846c79b3 100644 --- a/src/main/cpp/dbappender.cpp +++ b/src/main/cpp/dbappender.cpp @@ -109,7 +109,9 @@ DBAppender::~DBAppender() close(); } -void DBAppender::close(){ +void DBAppender::close() +{ + _priv->setClosed(); if(_priv->m_driver && _priv->m_databaseHandle){ apr_dbd_close(_priv->m_driver, _priv->m_databaseHandle); } diff --git a/src/main/cpp/nteventlogappender.cpp b/src/main/cpp/nteventlogappender.cpp index 42dbbf2c..ff6b57d1 100644 --- a/src/main/cpp/nteventlogappender.cpp +++ b/src/main/cpp/nteventlogappender.cpp @@ -133,6 +133,8 @@ NTEventLogAppender::~NTEventLogAppender() void NTEventLogAppender::close() { + priv->setClosed(); + if (priv->hEventLog != NULL) { ::DeregisterEventSource(priv->hEventLog); diff --git a/src/main/cpp/odbcappender.cpp b/src/main/cpp/odbcappender.cpp index b66d18d7..4616b2f8 100644 --- a/src/main/cpp/odbcappender.cpp +++ b/src/main/cpp/odbcappender.cpp @@ -392,6 +392,7 @@ void ODBCAppender::close() _priv->errorHandler->error(LOG4CXX_STR("Error closing connection"), e, ErrorCode::GENERIC_FAILURE); } + _priv->setClosed(); #if LOG4CXX_HAVE_ODBC @@ -407,7 +408,6 @@ void ODBCAppender::close() } #endif - _priv->closed = true; } #if LOG4CXX_HAVE_ODBC diff --git a/src/main/cpp/smtpappender.cpp b/src/main/cpp/smtpappender.cpp index 752d25db..fdf3c489 100644 --- a/src/main/cpp/smtpappender.cpp +++ b/src/main/cpp/smtpappender.cpp @@ -694,7 +694,7 @@ bool SMTPAppender::checkEntryConditions() void SMTPAppender::close() { - _priv->closed = true; + _priv->setClosed(); } LogString SMTPAppender::getTo() const diff --git a/src/main/cpp/socketappenderskeleton.cpp b/src/main/cpp/socketappenderskeleton.cpp index a0bdecfa..896c8061 100644 --- a/src/main/cpp/socketappenderskeleton.cpp +++ b/src/main/cpp/socketappenderskeleton.cpp @@ -235,7 +235,7 @@ void SocketAppenderSkeleton::retryConnect() void SocketAppenderSkeleton::SocketAppenderSkeletonPriv::stopMonitor() { - this->closed = true; + this->setClosed(); if (this->taskName.empty()) ; else if (auto pManager = this->taskManager.lock()) diff --git a/src/main/cpp/syslogappender.cpp b/src/main/cpp/syslogappender.cpp index aee1ae80..a7d746b4 100644 --- a/src/main/cpp/syslogappender.cpp +++ b/src/main/cpp/syslogappender.cpp @@ -69,7 +69,7 @@ SyslogAppender::~SyslogAppender() /** Release any resources held by this SyslogAppender.*/ void SyslogAppender::close() { - _priv->closed = true; + _priv->setClosed(); if (_priv->sw) { diff --git a/src/main/cpp/telnetappender.cpp b/src/main/cpp/telnetappender.cpp index 0da39124..40506ee0 100644 --- a/src/main/cpp/telnetappender.cpp +++ b/src/main/cpp/telnetappender.cpp @@ -82,12 +82,10 @@ struct TelnetAppender::TelnetAppenderPriv : public AppenderSkeletonPrivate void stopAcceptingConnections() { - { - std::lock_guard<std::recursive_mutex> lock(this->mutex); - if (!this->serverSocket || this->closed) - return; - this->closed = true; - } + if (!this->setClosed()) + return; + if (!this->serverSocket) + return; // Interrupt accept() try { diff --git a/src/main/cpp/writerappender.cpp b/src/main/cpp/writerappender.cpp index 1be53e87..7830fbd6 100644 --- a/src/main/cpp/writerappender.cpp +++ b/src/main/cpp/writerappender.cpp @@ -147,14 +147,11 @@ bool WriterAppender::WriterAppenderPriv::checkWriter() */ void WriterAppender::close() { - std::lock_guard<std::recursive_mutex> lock(_priv->mutex); - - if (_priv->closed) + if (!_priv->setClosed()) { return; } - _priv->closed = true; closeWriter(); } diff --git a/src/main/include/log4cxx/private/appenderskeleton_priv.h b/src/main/include/log4cxx/private/appenderskeleton_priv.h index cba7bec4..6c512b42 100644 --- a/src/main/include/log4cxx/private/appenderskeleton_priv.h +++ b/src/main/include/log4cxx/private/appenderskeleton_priv.h @@ -76,6 +76,11 @@ struct AppenderSkeleton::AppenderSkeletonPrivate bool warnedNoLayout = false; bool checkLayout(); + + /** + @returns true if the appender was not already closed + */ + bool setClosed(); }; } diff --git a/src/test/cpp/vectorappender.cpp b/src/test/cpp/vectorappender.cpp index e01dbad8..aa7999a3 100644 --- a/src/test/cpp/vectorappender.cpp +++ b/src/test/cpp/vectorappender.cpp @@ -16,6 +16,7 @@ */ #include "vectorappender.h" +#include <log4cxx/private/appenderskeleton_priv.h> #include <thread> using namespace log4cxx; @@ -30,12 +31,12 @@ void VectorAppender::append(const spi::LoggingEventPtr& event, Pool& /*p*/) this->vector.push_back(event); } -void VectorAppender::close() +bool VectorAppender::isClosed() const { - if (m_priv->closed) - { - return; - } + return m_priv->closed; +} +void VectorAppender::close() +{ m_priv->closed = true; } diff --git a/src/test/cpp/vectorappender.h b/src/test/cpp/vectorappender.h index 0d379fd4..25c47814 100644 --- a/src/test/cpp/vectorappender.h +++ b/src/test/cpp/vectorappender.h @@ -18,7 +18,6 @@ #include <log4cxx/appenderskeleton.h> #include <vector> #include <log4cxx/spi/loggingevent.h> -#include <log4cxx/private/appenderskeleton_priv.h> namespace LOG4CXX_NS { @@ -52,10 +51,7 @@ class VectorAppender : public AppenderSkeleton void close() override; - bool isClosed() const - { - return m_priv->closed; - } + bool isClosed() const; bool requiresLayout() const override {
