From 71a9bdafbded9fa73f4c8475509e319817d20dcb Mon Sep 17 00:00:00 2001 From: siren186 <765495939@qq.com> Date: Thu, 12 Sep 2024 18:26:49 +0800 Subject: [PATCH] 4557 ndc thread local (#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 --- .../include/Poco/NestedDiagnosticContext.h | 4 +- Foundation/src/NestedDiagnosticContext.cpp | 28 +++++++--- Foundation/testsuite/src/NDCTest.cpp | 55 ++++++++++++++++++- Foundation/testsuite/src/NDCTest.h | 2 + 4 files changed, 79 insertions(+), 10 deletions(-) diff --git a/Foundation/include/Poco/NestedDiagnosticContext.h b/Foundation/include/Poco/NestedDiagnosticContext.h index 87412d6c2..94d3ee075 100644 --- a/Foundation/include/Poco/NestedDiagnosticContext.h +++ b/Foundation/include/Poco/NestedDiagnosticContext.h @@ -89,9 +89,11 @@ public: /// 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. diff --git a/Foundation/src/NestedDiagnosticContext.cpp b/Foundation/src/NestedDiagnosticContext.cpp index 3275a9e6b..6dd98858a 100644 --- a/Foundation/src/NestedDiagnosticContext.cpp +++ b/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; } diff --git a/Foundation/testsuite/src/NDCTest.cpp b/Foundation/testsuite/src/NDCTest.cpp index 5891707d8..875b79783 100644 --- a/Foundation/testsuite/src/NDCTest.cpp +++ b/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 +#include using Poco::NDC; +using Poco::ActiveThreadPool; +using Poco::RunnableAdapter; +using Poco::Path; NDCTest::NDCTest(const std::string& name): CppUnit::TestCase(name) @@ -49,14 +57,39 @@ 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); } @@ -64,6 +97,25 @@ void NDCTest::testNDCScope() } +void NDCTest::testNDCMultiThread() +{ + ActiveThreadPool pool; + RunnableAdapter 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; } diff --git a/Foundation/testsuite/src/NDCTest.h b/Foundation/testsuite/src/NDCTest.h index a74bed15d..ce67731f8 100644 --- a/Foundation/testsuite/src/NDCTest.h +++ b/Foundation/testsuite/src/NDCTest.h @@ -26,6 +26,7 @@ public: void testNDC(); void testNDCScope(); + void testNDCMultiThread(); void setUp(); void tearDown(); @@ -33,6 +34,7 @@ public: static CppUnit::Test* suite(); private: + void runInThread(); };