Prevent undefined behaviour when an appender is deleted without being…

… closed (#400)
apache · Jul 25, 2024 · 1a8875a · 1a8875a
1 parent 6b61e97
commit 1a8875a
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 21 deletions.
diff --git 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,19 +104,22 @@ 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;
 	poll.p = _priv->pool.getAPRPool();
 	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
@@ -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
@@ -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
@@ -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
@@ -118,7 +118,6 @@ class TelnetAppenderTestCase : public AppenderSkeletonTestCase
 #endif
 				LOG4CXX_INFO(root, "Hello, World " << i);
 			}
-			appender->close();
 		}
 
 };