This is an automated email from the ASF dual-hosted git repository. swebb2066 pushed a commit to branch improve_message_buffer in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit 0378a6eacffff5e7d78142bd7918253f31c13e3e Author: Stephen Webb <[email protected]> AuthorDate: Thu Feb 12 17:01:00 2026 +1100 Prevent message loss when the calculation of a logged value also logs --- src/main/cpp/messagebuffer.cpp | 15 ++-- src/main/cpp/threadspecificdata.cpp | 86 ++++++++++++++++++++-- .../include/log4cxx/helpers/threadspecificdata.h | 14 +++- src/test/cpp/helpers/messagebuffertest.cpp | 46 ++++++++++++ 4 files changed, 144 insertions(+), 17 deletions(-) diff --git a/src/main/cpp/messagebuffer.cpp b/src/main/cpp/messagebuffer.cpp index df908bff..c34a74ba 100644 --- a/src/main/cpp/messagebuffer.cpp +++ b/src/main/cpp/messagebuffer.cpp @@ -22,13 +22,7 @@ #endif #include <log4cxx/helpers/messagebuffer.h> #include <log4cxx/helpers/transcoder.h> -#if !defined(LOG4CXX) - #define LOG4CXX 1 -#endif -#include <log4cxx/private/log4cxx_private.h> -#if !LOG4CXX_HAS_THREAD_LOCAL #include <log4cxx/helpers/threadspecificdata.h> -#endif using namespace LOG4CXX_NS::helpers; @@ -43,6 +37,11 @@ struct StringOrStream StringOrStream() : stream(nullptr) {} + ~StringOrStream() + { + if (this->stream) + ThreadSpecificData::releaseStringStream<T>(*this->stream); + } /** * Move the character buffer from \c buf to \c stream */ @@ -51,11 +50,7 @@ struct StringOrStream if (!this->stream) { const static std::basic_ostringstream<T> initialState; -#if LOG4CXX_HAS_THREAD_LOCAL - thread_local static std::basic_ostringstream<T> sStream; -#else auto& sStream = ThreadSpecificData::getStringStream<T>(); -#endif this->stream = &sStream; this->stream->clear(); this->stream->precision(initialState.precision()); diff --git a/src/main/cpp/threadspecificdata.cpp b/src/main/cpp/threadspecificdata.cpp index ffaf1566..1bd88bf8 100644 --- a/src/main/cpp/threadspecificdata.cpp +++ b/src/main/cpp/threadspecificdata.cpp @@ -33,6 +33,7 @@ #include <thread> #include <mutex> #include <list> +#include <array> using namespace LOG4CXX_NS; using namespace LOG4CXX_NS::helpers; @@ -49,14 +50,21 @@ struct ThreadSpecificData::ThreadSpecificDataPrivate{ std::shared_ptr<NamePair> pNamePair; + template <typename T> + struct CountedStringStream + { + int usage_count{ 0 }; + std::basic_ostringstream<T> ss; + }; + static const int max_depth = 3; #if !LOG4CXX_LOGCHAR_IS_UNICHAR && !LOG4CXX_LOGCHAR_IS_WCHAR - std::basic_ostringstream<logchar> logchar_stringstream; + std::array<CountedStringStream<logchar>, max_depth> logchar_stringstream; #endif #if LOG4CXX_WCHAR_T_API || LOG4CXX_LOGCHAR_IS_WCHAR - std::basic_ostringstream<wchar_t> wchar_stringstream; + std::array<CountedStringStream<wchar_t>, max_depth> wchar_stringstream; #endif #if LOG4CXX_UNICHAR_API || LOG4CXX_LOGCHAR_IS_UNICHAR - std::basic_ostringstream<UniChar> unichar_stringstream; + std::array<CountedStringStream<UniChar>, max_depth> unichar_stringstream; #endif void setThreadIdName(); @@ -166,21 +174,87 @@ auto ThreadSpecificData::getNames() -> NamePairPtr #if !LOG4CXX_LOGCHAR_IS_UNICHAR && !LOG4CXX_LOGCHAR_IS_WCHAR std::basic_ostringstream<logchar>& ThreadSpecificData::getStream(const logchar&) { - return getCurrentData()->m_priv->logchar_stringstream; + auto p = getCurrentData()->m_priv.get(); + auto pItem = &p->logchar_stringstream[0]; + for (auto& item : p->logchar_stringstream) + { + pItem = &item; + if (0 == item.usage_count) + break; + } + ++pItem->usage_count; + return pItem->ss; +} + +void ThreadSpecificData::releaseStream(std::basic_ostringstream<logchar>& ss) +{ + auto p = getCurrentData()->m_priv.get(); + for (auto& item : p->logchar_stringstream) + { + if (&item.ss == &ss) + { + --item.usage_count; + break; + } + } } #endif #if LOG4CXX_WCHAR_T_API || LOG4CXX_LOGCHAR_IS_WCHAR std::basic_ostringstream<wchar_t>& ThreadSpecificData::getStream(const wchar_t&) { - return getCurrentData()->m_priv->wchar_stringstream; + auto p = getCurrentData()->m_priv.get(); + auto pItem = &p->wchar_stringstream[0]; + for (auto& item : p->wchar_stringstream) + { + pItem = &item; + if (0 == item.usage_count) + break; + } + ++pItem->usage_count; + return pItem->ss; +} + +void ThreadSpecificData::releaseStream(std::basic_ostringstream<wchar_t>& ss) +{ + auto p = getCurrentData()->m_priv.get(); + for (auto& item : p->wchar_stringstream) + { + if (&item.ss == &ss) + { + --item.usage_count; + break; + } + } } #endif #if LOG4CXX_UNICHAR_API || LOG4CXX_LOGCHAR_IS_UNICHAR std::basic_ostringstream<UniChar>& ThreadSpecificData::getStream(const UniChar&) { - return getCurrentData()->m_priv->unichar_stringstream; + auto p = getCurrentData()->m_priv.get(); + auto pItem = &p->unichar_stringstream[0]; + for (auto& item : p->unichar_stringstream) + { + pItem = &item; + if (0 == item.usage_count) + break; + } + ++pItem->usage_count; + return pItem->ss; +} + +void ThreadSpecificData::releaseStream(std::basic_ostringstream<UniChar>& ss) +{ + auto p = getCurrentData()->m_priv.get(); + for (auto& item : p->unichar_stringstream) + { + if (&item.ss == &ss) + { + --item.usage_count; + break; + } + } } #endif diff --git a/src/main/include/log4cxx/helpers/threadspecificdata.h b/src/main/include/log4cxx/helpers/threadspecificdata.h index f42d29fb..e9a7d6b3 100644 --- a/src/main/include/log4cxx/helpers/threadspecificdata.h +++ b/src/main/include/log4cxx/helpers/threadspecificdata.h @@ -73,7 +73,7 @@ class LOG4CXX_EXPORT ThreadSpecificData MDC::Map& getMap(); /** - * A character outpur stream only assessable to the current thread + * A character output stream only assessable to the current thread */ template <typename T> static std::basic_ostringstream<T>& getStringStream() @@ -81,6 +81,15 @@ class LOG4CXX_EXPORT ThreadSpecificData return getStream(T()); } + /** + * Make \c ss available for use by the current thread + */ + template <typename T> + static void releaseStringStream(std::basic_ostringstream<T>& ss) + { + return releaseStream(ss); + } + /** * The names assigned to the current thread */ @@ -100,12 +109,15 @@ class LOG4CXX_EXPORT ThreadSpecificData private: #if !LOG4CXX_LOGCHAR_IS_UNICHAR && !LOG4CXX_LOGCHAR_IS_WCHAR static std::basic_ostringstream<logchar>& getStream(const logchar&); + static void releaseStream(std::basic_ostringstream<logchar>&); #endif #if LOG4CXX_WCHAR_T_API || LOG4CXX_LOGCHAR_IS_WCHAR static std::basic_ostringstream<wchar_t>& getStream(const wchar_t&); + static void releaseStream(std::basic_ostringstream<wchar_t>&); #endif #if LOG4CXX_UNICHAR_API || LOG4CXX_LOGCHAR_IS_UNICHAR static std::basic_ostringstream<UniChar>& getStream(const UniChar&); + static void releaseStream(std::basic_ostringstream<UniChar>&); #endif LOG4CXX_DECLARE_PRIVATE_MEMBER_PTR(ThreadSpecificDataPrivate, m_priv) }; diff --git a/src/test/cpp/helpers/messagebuffertest.cpp b/src/test/cpp/helpers/messagebuffertest.cpp index e257528d..9b164b35 100644 --- a/src/test/cpp/helpers/messagebuffertest.cpp +++ b/src/test/cpp/helpers/messagebuffertest.cpp @@ -24,6 +24,7 @@ #include <log4cxx/logger.h> #include <log4cxx/propertyconfigurator.h> #include "util/compare.h" +#include <cassert> #if LOG4CXX_CFSTRING_API #include <CoreFoundation/CFString.h> @@ -58,6 +59,7 @@ LOGUNIT_CLASS(MessageBufferTest) #if LOG4CXX_CFSTRING_API LOGUNIT_TEST(testInsertCFString); #endif + LOGUNIT_TEST(testInsertCalculatedValue); LOGUNIT_TEST_SUITE_END(); @@ -256,6 +258,50 @@ public: } #endif + // Use the Gregory-Leibniz series to approximate π + static double calculatePi(int terms, int level, int initialTerm = 0) + { + double pi = 0.0; + if (0 < level) + { + MessageBuffer buf; + auto& retval = buf << "level " << level << " pi " << std::setprecision(6) << (pi = calculatePi(terms * 2, level - 1, terms)); + assert(buf.hasStream()); + helpers::LogLog::debug(buf.str(retval)); + pi /= 4; // Divide by 4 to get the value of the previous level + } + for (int i = initialTerm; i < terms; i++) + { + if (i % 2 == 0) + { + pi += 1.0 / (2 * i + 1); // Add for even index + } + else + { + pi -= 1.0 / (2 * i + 1); // Subtract for odd index + } + if ((i + 1) % 500 == 0) + { + MessageBuffer buf; + auto& retval = buf << "level " << level << " term " << i << " pi " << std::setprecision(6) << (pi * 4); + assert(buf.hasStream()); + helpers::LogLog::debug(buf.str(retval)); + } + } + return pi * 4; // Multiply by 4 to get Pi + } + + // Checks what happens with 4 concurrently active MessageBuffer objects in the same thread, each using using a std::stringstream. + // The Log4cxx debug output shows the partially completed message (at the 3rd level) being reset for use by the 4th level MessageBuffer. + void testInsertCalculatedValue() + { + MessageBuffer buf; + std::string expectedValue("pi=3.142 calculated pi=3.141 using 4000 terms"); + auto& retval = buf << "pi=" << std::setprecision(4) << 3.14159265358979323846 << " calculated pi=" << std::setprecision(4) << calculatePi(1000, 2) << " using 4000 terms"; + LOGUNIT_ASSERT_EQUAL(true, buf.hasStream()); + LOGUNIT_ASSERT_EQUAL(expectedValue, buf.str(retval)); + } + }; LOGUNIT_TEST_SUITE_REGISTRATION(MessageBufferTest);
