Prevent abnormal termination on exit when using a static NDC/MDC vari…

…able (#426) * Add a test of the effect of a static NDC variable
apache · Nov 7, 2024 · a568d0f · a568d0f
1 parent 4642a50
commit a568d0f
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 36 deletions.
diff --git a/src/main/cpp/loggingevent.cpp b/src/main/cpp/loggingevent.cpp
@@ -38,7 +38,7 @@ using namespace LOG4CXX_NS::helpers;
 
 struct LoggingEvent::LoggingEventPrivate
 {
-	LoggingEventPrivate(const ThreadSpecificData::NamePairPtr& p = ThreadSpecificData::getNames()) :
+	LoggingEventPrivate(const ThreadSpecificData::NamePairPtr p = ThreadSpecificData::getNames()) :
 		timeStamp(0),
 #if LOG4CXX_ABI_VERSION <= 15
 		threadName(p->idString),
@@ -53,7 +53,7 @@ struct LoggingEvent::LoggingEventPrivate
 		, const LevelPtr& level1
 		, const LocationInfo& locationInfo1
 		, LogString&& message1
-		, const ThreadSpecificData::NamePairPtr& p = ThreadSpecificData::getNames()
+		, const ThreadSpecificData::NamePairPtr p = ThreadSpecificData::getNames()
 		) :
 		logger(logger1),
 		level(level1),
@@ -243,19 +243,24 @@ LoggingEvent::KeySet LoggingEvent::getMDCKeySet() const
 		for (auto const& item : m_priv->dc->map)
 			result.push_back(item.first);
 	}
-	else for (auto const& item : ThreadSpecificData::getCurrentData()->getMap())
-		result.push_back(item.first);
+	else if (auto pData = ThreadSpecificData::getCurrentData())
+	{
+		for (auto const& item : pData->getMap())
+			result.push_back(item.first);
+	}
 	return result;
 }
 
 void LoggingEvent::LoadDC() const
 {
 	m_priv->dc = std::make_unique<LoggingEventPrivate::DiagnosticContext>();
-	auto pData = ThreadSpecificData::getCurrentData();
-	m_priv->dc->map = pData->getMap();
-	auto& stack = pData->getStack();
-	if (!stack.empty())
-		m_priv->dc->ctx = stack.top();
+	if (auto pData = ThreadSpecificData::getCurrentData())
+	{
+		m_priv->dc->map = pData->getMap();
+		auto& stack = pData->getStack();
+		if (!stack.empty())
+			m_priv->dc->ctx = stack.top();
+	}
 }
 
 #if LOG4CXX_ABI_VERSION <= 15

diff --git a/src/main/cpp/threadspecificdata.cpp b/src/main/cpp/threadspecificdata.cpp
@@ -142,6 +142,7 @@ ThreadSpecificData::ThreadSpecificData(ThreadSpecificData&& other)
 
 ThreadSpecificData::~ThreadSpecificData()
 {
+	m_priv.reset();
 }
 
 NDC::Stack& ThreadSpecificData::getStack()
@@ -156,7 +157,8 @@ MDC::Map& ThreadSpecificData::getMap()
 
 auto ThreadSpecificData::getNames() -> NamePairPtr
 {
-	return getCurrentData()->m_priv->pNamePair;
+	auto p = getCurrentData();
+	return p ? p->m_priv->pNamePair : std::make_shared<NamePair>();
 }
 
 #if !LOG4CXX_LOGCHAR_IS_UNICHAR && !LOG4CXX_LOGCHAR_IS_WCHAR
@@ -184,7 +186,7 @@ ThreadSpecificData* ThreadSpecificData::getCurrentData()
 {
 #if LOG4CXX_HAS_THREAD_LOCAL
 	thread_local ThreadSpecificData data;
-	return &data;
+	return data.m_priv ? &data : NULL;
 #elif APR_HAS_THREADS
 	void* pData = NULL;
 	if (APR_SUCCESS == apr_threadkey_private_get(&pData, APRInitializer::getTlsKey())
@@ -230,12 +232,16 @@ void ThreadSpecificData::recycle()
 
 void ThreadSpecificData::put(const LogString& key, const LogString& val)
 {
-	getCurrentData()->getMap()[key] = val;
+	if (auto p = getCurrentData())
+		p->getMap()[key] = val;
 }
 
 void ThreadSpecificData::push(const LogString& val)
 {
-	NDC::Stack& stack = getCurrentData()->getStack();
+	auto p = getCurrentData();
+	if (!p)
+		return;
+	NDC::Stack& stack = p->getStack();
 	if (stack.empty())
 	{
 		stack.push(NDC::DiagnosticContext(val, val));
@@ -251,6 +257,7 @@ void ThreadSpecificData::push(const LogString& val)
 
 void ThreadSpecificData::inherit(const NDC::Stack& src)
 {
-	getCurrentData()->getStack() = src;
+	if (auto p = getCurrentData())
+		p->getStack() = src;
 }
 
diff --git a/src/main/include/log4cxx/helpers/threadspecificdata.h b/src/main/include/log4cxx/helpers/threadspecificdata.h
@@ -38,7 +38,7 @@ class LOG4CXX_EXPORT ThreadSpecificData
 
 		/**
 		 *  Gets current thread specific data.
-		 *  @return a pointer that is never null.
+		 *  @return a pointer that is non-null prior to application exit.
 		 */
 		static ThreadSpecificData* getCurrentData();
 

diff --git a/src/test/cpp/ndctestcase.cpp b/src/test/cpp/ndctestcase.cpp
@@ -24,7 +24,7 @@
 #include "logunit.h"
 #include "util/compare.h"
 
-
+static log4cxx::NDC ndc("ndctest");
 
 using namespace log4cxx;
 

diff --git a/src/test/resources/witness/ndc/NDC.1 b/src/test/resources/witness/ndc/NDC.1
@@ -1,23 +1,23 @@
-DEBUG null - m1
-INFO  null - m2
-WARN  null - m3
-ERROR null - m4
-FATAL null - m5
-DEBUG n1 - m1
-INFO  n1 - m2
-WARN  n1 - m3
-ERROR n1 - m4
-FATAL n1 - m5
-DEBUG n1 n2 n3 - m1
-INFO  n1 n2 n3 - m2
-WARN  n1 n2 n3 - m3
-ERROR n1 n2 n3 - m4
-FATAL n1 n2 n3 - m5
-DEBUG n1 n2 - m1
-INFO  n1 n2 - m2
-WARN  n1 n2 - m3
-ERROR n1 n2 - m4
-FATAL n1 n2 - m5
+DEBUG ndctest - m1
+INFO  ndctest - m2
+WARN  ndctest - m3
+ERROR ndctest - m4
+FATAL ndctest - m5
+DEBUG ndctest n1 - m1
+INFO  ndctest n1 - m2
+WARN  ndctest n1 - m3
+ERROR ndctest n1 - m4
+FATAL ndctest n1 - m5
+DEBUG ndctest n1 n2 n3 - m1
+INFO  ndctest n1 n2 n3 - m2
+WARN  ndctest n1 n2 n3 - m3
+ERROR ndctest n1 n2 n3 - m4
+FATAL ndctest n1 n2 n3 - m5
+DEBUG ndctest n1 n2 - m1
+INFO  ndctest n1 n2 - m2
+WARN  ndctest n1 n2 - m3
+ERROR ndctest n1 n2 - m4
+FATAL ndctest n1 n2 - m5
 DEBUG null - m1
 INFO  null - m2
 WARN  null - m3