From 1a8875ad839a37d2fcf31876c0bbfe61181c9f21 Mon Sep 17 00:00:00 2001
From: Stephen Webb <stephen.webb@ieee.org>
Date: Fri, 26 Jul 2024 09:21:42 +1000
Subject: [PATCH] Prevent undefined behaviour when an appender is deleted
 without being closed (#400)

---
 src/main/cpp/aprserversocket.cpp               | 18 +++++++++++-------
 src/main/cpp/filewatchdog.cpp                  |  2 +-
 src/main/cpp/telnetappender.cpp                | 18 +++++++++---------
 .../private/socketappenderskeleton_priv.h      |  6 +++---
 src/test/cpp/net/telnetappendertestcase.cpp    |  1 -
 5 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/src/main/cpp/aprserversocket.cpp b/src/main/cpp/aprserversocket.cpp
index 080a6a29a..580f757f0 100644
--- a/src/main/cpp/aprserversocket.cpp
+++ b/src/main/cpp/aprserversocket.cpp
@@ -81,7 +81,8 @@ APRServerSocket::APRServerSocket(int port) :
 	}
 }
 
-void APRServerSocket::close(){
+void APRServerSocket::close()
+{
 	std::lock_guard<std::mutex> lock(_priv->mutex);
 
 	if (_priv->socket != 0)
@@ -103,11 +104,14 @@ accepts it
 */
 SocketPtr APRServerSocket::accept()
 {
-	std::lock_guard<std::mutex> lock(_priv->mutex);
-
-	if (_priv->socket == 0)
+	apr_socket_t* s;
+	{
+		std::lock_guard<std::mutex> lock(_priv->mutex);
+		s = _priv->socket;
+	}
+	if (s == 0)
 	{
-		throw IOException();
+		throw IOException(LOG4CXX_STR("socket is 0"));
 	}
 
 	apr_pollfd_t poll;
@@ -115,7 +119,7 @@ SocketPtr APRServerSocket::accept()
 	poll.desc_type = APR_POLL_SOCKET;
 	poll.reqevents = APR_POLLIN;
 	poll.rtnevents = 0;
-	poll.desc.s = _priv->socket;
+	poll.desc.s = s;
 	poll.client_data = NULL;
 
 	apr_int32_t signaled;
@@ -140,7 +144,7 @@ SocketPtr APRServerSocket::accept()
 	}
 
 	apr_socket_t* newSocket;
-	status = apr_socket_accept(&newSocket, _priv->socket, newPool);
+	status = apr_socket_accept(&newSocket, s, newPool);
 
 	if (status != APR_SUCCESS)
 	{
diff --git a/src/main/cpp/filewatchdog.cpp b/src/main/cpp/filewatchdog.cpp
index 351358be6..ef9f80cde 100644
--- a/src/main/cpp/filewatchdog.cpp
+++ b/src/main/cpp/filewatchdog.cpp
@@ -41,7 +41,7 @@ struct FileWatchdog::FileWatchdogPrivate{
 #if LOG4CXX_EVENTS_AT_EXIT
 		, atExitRegistryRaii([this]{stopWatcher();})
 #endif
-        {}
+        { stopWatcher(); }
 	/**
 	The name of the file to observe  for changes.
 	*/
diff --git a/src/main/cpp/telnetappender.cpp b/src/main/cpp/telnetappender.cpp
index 09d33def2..204aba305 100644
--- a/src/main/cpp/telnetappender.cpp
+++ b/src/main/cpp/telnetappender.cpp
@@ -52,7 +52,7 @@ struct TelnetAppender::TelnetAppenderPriv : public AppenderSkeletonPrivate
 #if LOG4CXX_EVENTS_AT_EXIT
 		, atExitRegistryRaii([this]{stopAcceptingConnections();})
 #endif
-        {}
+        { stopAcceptingConnections(); }
 
 	int port;
 	ConnectionList connections;
@@ -73,14 +73,14 @@ struct TelnetAppender::TelnetAppenderPriv : public AppenderSkeletonPrivate
 			if (!this->serverSocket || this->closed)
 				return;
 			this->closed = true;
-			// Interrupt accept()
-			try
-			{
-				this->serverSocket->close();
-			}
-			catch (Exception&)
-			{
-			}
+		}
+		// Interrupt accept()
+		try
+		{
+			this->serverSocket->close();
+		}
+		catch (Exception&)
+		{
 		}
 		if (this->sh.joinable())
 			this->sh.join();
diff --git a/src/main/include/log4cxx/private/socketappenderskeleton_priv.h b/src/main/include/log4cxx/private/socketappenderskeleton_priv.h
index 537a107fa..9519db1a1 100644
--- a/src/main/include/log4cxx/private/socketappenderskeleton_priv.h
+++ b/src/main/include/log4cxx/private/socketappenderskeleton_priv.h
@@ -42,7 +42,7 @@ struct SocketAppenderSkeleton::SocketAppenderSkeletonPriv : public AppenderSkele
 #if LOG4CXX_EVENTS_AT_EXIT
 		, atExitRegistryRaii([this]{stopMonitor();})
 #endif
-        {}
+        { stopMonitor(); }
 
 	SocketAppenderSkeletonPriv(helpers::InetAddressPtr address, int defaultPort, int reconnectionDelay) :
 		AppenderSkeletonPrivate(),
@@ -54,7 +54,7 @@ struct SocketAppenderSkeleton::SocketAppenderSkeletonPriv : public AppenderSkele
 #if LOG4CXX_EVENTS_AT_EXIT
 		, atExitRegistryRaii([this]{stopMonitor();})
 #endif
-        {}
+        { stopMonitor(); }
 
 	SocketAppenderSkeletonPriv(const LogString& host, int port, int delay) :
 		AppenderSkeletonPrivate(),
@@ -66,7 +66,7 @@ struct SocketAppenderSkeleton::SocketAppenderSkeletonPriv : public AppenderSkele
 #if LOG4CXX_EVENTS_AT_EXIT
 		, atExitRegistryRaii([this]{stopMonitor();})
 #endif
-        {}
+        { stopMonitor(); }
 
 	/**
 	host name
diff --git a/src/test/cpp/net/telnetappendertestcase.cpp b/src/test/cpp/net/telnetappendertestcase.cpp
index 1268c227a..b7aebbe57 100644
--- a/src/test/cpp/net/telnetappendertestcase.cpp
+++ b/src/test/cpp/net/telnetappendertestcase.cpp
@@ -118,7 +118,6 @@ class TelnetAppenderTestCase : public AppenderSkeletonTestCase
 #endif
 				LOG4CXX_INFO(root, "Hello, World " << i);
 			}
-			appender->close();
 		}
 
 };