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 fe6667df Prevent undefined behavior during a configuration change
(#601)
fe6667df is described below
commit fe6667df3600f2e3cc3f658c7edc33b3bf879640
Author: Stephen Webb <[email protected]>
AuthorDate: Sat Mar 7 10:53:03 2026 +1100
Prevent undefined behavior during a configuration change (#601)
---
src/main/cpp/appenderskeleton.cpp | 8 ++++++++
src/main/cpp/dbappender.cpp | 4 +++-
src/main/cpp/nteventlogappender.cpp | 22 +++++++++++++++-------
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 +----
.../log4cxx/private/appenderskeleton_priv.h | 5 +++++
src/test/cpp/vectorappender.cpp | 11 ++++++-----
src/test/cpp/vectorappender.h | 6 +-----
12 files changed, 47 insertions(+), 32 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..e00ff4a4 100644
--- a/src/main/cpp/nteventlogappender.cpp
+++ b/src/main/cpp/nteventlogappender.cpp
@@ -50,6 +50,8 @@ struct NTEventLogAppender::NTEventLogAppenderPrivate : public
AppenderSkeleton::
LogString source;
HANDLE hEventLog;
SID* pCurrentUserSID;
+
+ void close();
};
class CCtUserSIDHelper
@@ -133,16 +135,22 @@ NTEventLogAppender::~NTEventLogAppender()
void NTEventLogAppender::close()
{
- if (priv->hEventLog != NULL)
+ priv->setClosed();
+ priv->close();
+}
+
+void NTEventLogAppender::NTEventLogAppenderPrivate::close()
+{
+ if (this->hEventLog != NULL)
{
- ::DeregisterEventSource(priv->hEventLog);
- priv->hEventLog = NULL;
+ ::DeregisterEventSource(this->hEventLog);
+ this->hEventLog = NULL;
}
- if (priv->pCurrentUserSID != NULL)
+ if (this->pCurrentUserSID != NULL)
{
- CCtUserSIDHelper::FreeSid((::SID*) priv->pCurrentUserSID);
- priv->pCurrentUserSID = NULL;
+ CCtUserSIDHelper::FreeSid((::SID*) this->pCurrentUserSID);
+ this->pCurrentUserSID = NULL;
}
}
@@ -181,7 +189,7 @@ void NTEventLogAppender::activateOptions(Pool&)
priv->log = LOG4CXX_STR("Application");
}
- close();
+ priv->close();
// current user security identifier
CCtUserSIDHelper::GetCurrentUserSID((::SID**) &priv->pCurrentUserSID);
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
{
