[logging-log4cxx] 01/01: LOGCXX-537 avoid deadlock if socket fails

[rm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rmiddleton pushed a commit to branch LOGCXX-537
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git

commit d3ce7942082e918ed2a38ffab9a8523b19d6a60a
Author: Robert Middleton <ro...@rm5248.com>
AuthorDate: Sat Dec 11 18:22:40 2021 -0500

    LOGCXX-537 avoid deadlock if socket fails
---
 src/main/cpp/socketappender.cpp                    |  2 +-
 src/main/cpp/socketappenderskeleton.cpp            |  4 ++
 src/main/cpp/xmlsocketappender.cpp                 |  2 +-
 .../include/log4cxx/net/socketappenderskeleton.h   |  5 +++
 src/test/cpp/net/socketappendertestcase.cpp        | 49 ++++++++++++++++++++++
 5 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/src/main/cpp/socketappender.cpp b/src/main/cpp/socketappender.cpp
index 57f8ade..5f9fb8b 100644
--- a/src/main/cpp/socketappender.cpp
+++ b/src/main/cpp/socketappender.cpp
@@ -124,7 +124,7 @@ void SocketAppender::append(const spi::LoggingEventPtr& event, log4cxx::helpers:
 
 		if (getReconnectionDelay() > 0)
 		{
-			fireConnector();
+			fireConnectorInternal();
 		}
 	}
 }
diff --git a/src/main/cpp/socketappenderskeleton.cpp b/src/main/cpp/socketappenderskeleton.cpp
index e3e1375..27f9643 100644
--- a/src/main/cpp/socketappenderskeleton.cpp
+++ b/src/main/cpp/socketappenderskeleton.cpp
@@ -159,6 +159,10 @@ void SocketAppenderSkeleton::fireConnector()
 {
 	std::unique_lock<log4cxx::shared_mutex> lock(mutex);
 
+	fireConnectorInternal();
+}
+
+void SocketAppenderSkeleton::fireConnectorInternal(){
 	if ( !thread.joinable() )
 	{
 		LogLog::debug(LOG4CXX_STR("Connector thread not alive: starting monitor."));
diff --git a/src/main/cpp/xmlsocketappender.cpp b/src/main/cpp/xmlsocketappender.cpp
index d5b9942..4c1c70d 100644
--- a/src/main/cpp/xmlsocketappender.cpp
+++ b/src/main/cpp/xmlsocketappender.cpp
@@ -122,7 +122,7 @@ void XMLSocketAppender::append(const spi::LoggingEventPtr& event, log4cxx::helpe
 
 			if (getReconnectionDelay() > 0)
 			{
-				fireConnector();
+				fireConnectorInternal();
 			}
 		}
 	}
diff --git a/src/main/include/log4cxx/net/socketappenderskeleton.h b/src/main/include/log4cxx/net/socketappenderskeleton.h
index 88d2a15..8020613 100644
--- a/src/main/include/log4cxx/net/socketappenderskeleton.h
+++ b/src/main/include/log4cxx/net/socketappenderskeleton.h
@@ -178,6 +178,11 @@ class LOG4CXX_EXPORT SocketAppenderSkeleton : public AppenderSkeleton
 
 		virtual int getDefaultPort() const = 0;
 
+		/**
+		 * Fire the connector.  Caller must have mutex locked.
+		 */
+		void fireConnectorInternal();
+
 	private:
 		void connect(log4cxx::helpers::Pool& p);
 		/**
diff --git a/src/test/cpp/net/socketappendertestcase.cpp b/src/test/cpp/net/socketappendertestcase.cpp
index fe3753c..66d74fc 100644
--- a/src/test/cpp/net/socketappendertestcase.cpp
+++ b/src/test/cpp/net/socketappendertestcase.cpp
@@ -16,6 +16,9 @@
  */
 
 #include <log4cxx/net/socketappender.h>
+#include <log4cxx/patternlayout.h>
+#include <log4cxx/basicconfigurator.h>
+#include <log4cxx/helpers/serversocket.h>
 #include "../appenderskeletontestcase.h"
 #include "apr.h"
 
@@ -34,16 +37,62 @@ class SocketAppenderTestCase : public AppenderSkeletonTestCase
 		//
 		LOGUNIT_TEST(testDefaultThreshold);
 		LOGUNIT_TEST(testSetOptionThreshold);
+		LOGUNIT_TEST(testInvalidHost);
 
 		LOGUNIT_TEST_SUITE_END();
 
 
 	public:
 
+		void setUp()
+		{
+		}
+
+		void tearDown()
+		{
+			BasicConfigurator::resetConfiguration();
+		}
+
 		AppenderSkeleton* createAppenderSkeleton() const
 		{
 			return new log4cxx::net::SocketAppender();
 		}
+
+		void testInvalidHost(){
+			log4cxx::net::SocketAppenderPtr appender = std::make_shared<log4cxx::net::SocketAppender>();
+			log4cxx::PatternLayoutPtr layout = std::make_shared<log4cxx::PatternLayout>("%m%n");
+
+			log4cxx::helpers::ServerSocket serverSocket(4445);
+
+			appender->setLayout(layout);
+			appender->setRemoteHost(LOG4CXX_STR("localhost"));
+			appender->setReconnectionDelay(1);
+			appender->setPort(4445);
+			log4cxx::helpers::Pool pool;
+			appender->activateOptions(pool);
+
+			BasicConfigurator::configure(appender);
+
+			log4cxx::Logger::getRootLogger()->setLevel(log4cxx::Level::getAll());
+
+			std::thread th1( [](){
+				for( int x = 0; x < 3000; x++ ){
+					LOG4CXX_INFO(Logger::getLogger(LOG4CXX_STR("test")), "Some message" );
+				}
+			});
+			std::thread th2( [](){
+				for( int x = 0; x < 3000; x++ ){
+					LOG4CXX_INFO(Logger::getLogger(LOG4CXX_STR("test")), "Some message" );
+				}
+			});
+
+			SocketPtr incomingSocket = serverSocket.accept();
+			incomingSocket->close();
+
+			// If we do not get here, we have deadlocked
+			th1.join();
+			th2.join();
+		}
 };
 
 LOGUNIT_TEST_SUITE_REGISTRATION(SocketAppenderTestCase);