4557 ndc thread local (pocoproject#4682)

* fix(NestedDiagnosticContext): NDC crashed in multi-thread environment

* fix(NestedDiagnosticContext): TestCase output redirect

* enh(NestedDiagnosticContext): replace Poco::ThreadLocal to C++ standard thread_local so that objects can dtor when thread exit

* enh(NestedDiagnosticContext): remove unused header files

* chore(NDCTest): verify dump content

* chore(NDCTest): use __FILE__ macro

* fix(NDCTest): fix codeql warning

* fix(NDCTest): remove temp code

* enh(NestedDiagnosticContext): add nameOnly for dump

---------

Co-authored-by: Alex Fabijanic <alex@pocoproject.org>

Foundation/include/Poco/NestedDiagnosticContext.h

@@ -89,9 +89,11 @@ class Foundation_API NestedDiagnosticContext
 		/// to the given stream. The entries are delimited by
 		/// a newline.
 
-	void dump(std::ostream& ostr, const std::string& delimiter) const;
+	void dump(std::ostream& ostr, const std::string& delimiter, bool nameOnly = false) const;
 		/// Dumps the stack (including line number and filenames)
 		/// to the given stream.
+		/// If nameOnly is false (default), the whole path to file is printed,
+		/// otherwise only the file name.
 
 	void clear();
 		/// Clears the NDC stack.

Foundation/src/NestedDiagnosticContext.cpp

@@ -13,7 +13,7 @@
 
 
 #include "Poco/NestedDiagnosticContext.h"
-#include "Poco/ThreadLocal.h"
+#include "Poco/Path.h"
 
 
 namespace Poco {
@@ -95,14 +95,26 @@ void NestedDiagnosticContext::dump(std::ostream& ostr) const
 }
 
 
-void NestedDiagnosticContext::dump(std::ostream& ostr, const std::string& delimiter) const
+void NestedDiagnosticContext::dump(std::ostream& ostr, const std::string& delimiter, bool nameOnly) const
 {
-	for (const auto& i: _stack)
+	for (auto it = _stack.begin(); it != _stack.end(); ++it)
 	{
-		ostr << i.info;
-		if (i.file)
-			ostr << " (in \"" << i.file << "\", line " << i.line << ")";
-		ostr << delimiter;
+		if (it != _stack.begin())
+		{
+			ostr << delimiter;
+		}
+
+		std::string file = it->file ? it->file : "";
+		if (nameOnly && !file.empty())
+		{
+			file = Path(file).getFileName();
+		}
+
+		ostr << it->info;
+		if (!file.empty())
+		{
+			ostr << " (in \"" << file << "\", line " << it->line << ")";
+		}
 	}
 }
 
@@ -115,7 +127,7 @@ void NestedDiagnosticContext::clear()
 
 NestedDiagnosticContext& NestedDiagnosticContext::current()
 {
-	static NestedDiagnosticContext ndc;
+	static thread_local NestedDiagnosticContext ndc;
 	return ndc;
 }
 

Foundation/testsuite/src/NDCTest.cpp

@@ -12,10 +12,18 @@
 #include "CppUnit/TestCaller.h"
 #include "CppUnit/TestSuite.h"
 #include "Poco/NestedDiagnosticContext.h"
+#include "Poco/ActiveThreadPool.h"
+#include "Poco/RunnableAdapter.h"
+#include "Poco/Format.h"
+#include "Poco/Path.h"
 #include <iostream>
+#include <sstream>
 
 
 using Poco::NDC;
+using Poco::ActiveThreadPool;
+using Poco::RunnableAdapter;
+using Poco::Path;
 
 
 NDCTest::NDCTest(const std::string& name): CppUnit::TestCase(name)
@@ -49,21 +57,65 @@ void NDCTest::testNDC()
 void NDCTest::testNDCScope()
 {
 	poco_ndc("item1");
+	auto line1 = __LINE__ - 1;
 	assertTrue (NDC::current().depth() == 1);
+
 	{
 		poco_ndc("item2");
+		auto line2 = __LINE__ - 1;
 		assertTrue (NDC::current().depth() == 2);
+
 		{
 			poco_ndc("item3");
+			auto line3 = __LINE__ - 1;
 			assertTrue (NDC::current().depth() == 3);
-			NDC::current().dump(std::cout);
+
+ 			std::ostringstream ostr1;
+			NDC::current().dump(ostr1);
+			assertEqual (ostr1.str(), Poco::format(
+				"\"item1\" (in \"%s\", line %d)\n"
+				"\"item2\" (in \"%s\", line %d)\n"
+				"\"item3\" (in \"%s\", line %d)",
+				std::string(__FILE__), line1,
+				std::string(__FILE__), line2,
+				std::string(__FILE__), line3));
+
+			std::ostringstream ostr2;
+			NDC::current().dump(ostr2, "\n", true);
+			std::string fileName = Path(__FILE__).getFileName();
+			assertEqual(ostr2.str(), Poco::format(
+				"\"item1\" (in \"%s\", line %d)\n"
+				"\"item2\" (in \"%s\", line %d)\n"
+				"\"item3\" (in \"%s\", line %d)",
+				fileName, line1,
+				fileName, line2,
+				fileName, line3));
 		}
 		assertTrue (NDC::current().depth() == 2);
 	}
 	assertTrue (NDC::current().depth() == 1);
 }
 
 
+void NDCTest::testNDCMultiThread()
+{
+	ActiveThreadPool pool;
+	RunnableAdapter<NDCTest> ra(*this, &NDCTest::runInThread);
+	for (int i = 0; i < 1000; i++)
+	{
+		pool.start(ra);
+	}
+	pool.joinAll();
+}
+
+
+void NDCTest::runInThread()
+{
+	testNDC();
+	testNDCScope();
+}
+
+
 void NDCTest::setUp()
 {
 }
@@ -80,6 +132,7 @@ CppUnit::Test* NDCTest::suite()
 
 	CppUnit_addTest(pSuite, NDCTest, testNDC);
 	CppUnit_addTest(pSuite, NDCTest, testNDCScope);
+	CppUnit_addTest(pSuite, NDCTest, testNDCMultiThread);
 
 	return pSuite;
 }

Foundation/testsuite/src/NDCTest.h

@@ -26,13 +26,15 @@ class NDCTest: public CppUnit::TestCase
 
 	void testNDC();
 	void testNDCScope();
+	void testNDCMultiThread();
 
 	void setUp();
 	void tearDown();
 
 	static CppUnit::Test* suite();
 
 private:
+	void runInThread();
 };