From f7e7fa0983e83b922cb6336ff3a55d33b2873943 Mon Sep 17 00:00:00 2001 From: Duncan Horn <40036384+dunhor@users.noreply.github.com> Date: Thu, 25 Sep 2025 04:31:49 -0700 Subject: [PATCH] Fix ReusableStringStream to access the container under the lock (#3031) Commit 582200a made `ReusableStringStream`'s index reservation thread safe, however it's still accessing the `m_streams` vector outside the lock. This makes it so that the `add` call returns the pointer in addition to the index so that the lock doesn't need to get acquired again until destruction. The issue with accessing `m_streams` outside the lock is that `add` can call `push_back` on the vector, which might re-allocate. If this re-allocation occurs concurrently with anther thread trying to index into this array, you get UB (typically a null pointer read). --- .../internal/catch_reusable_string_stream.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/catch2/internal/catch_reusable_string_stream.cpp b/src/catch2/internal/catch_reusable_string_stream.cpp index a2dd5a63..875da4da 100644 --- a/src/catch2/internal/catch_reusable_string_stream.cpp +++ b/src/catch2/internal/catch_reusable_string_stream.cpp @@ -12,6 +12,7 @@ #include #include +#include #include namespace Catch { @@ -23,16 +24,16 @@ namespace Catch { std::ostringstream m_referenceStream; // Used for copy state/ flags from Detail::Mutex m_mutex; - auto add() -> std::size_t { + auto add() -> std::pair { Detail::LockGuard _( m_mutex ); if( m_unused.empty() ) { m_streams.push_back( Detail::make_unique() ); - return m_streams.size()-1; + return { m_streams.size()-1, m_streams.back().get() }; } else { auto index = m_unused.back(); m_unused.pop_back(); - return index; + return { index, m_streams[index].get() }; } } @@ -46,10 +47,10 @@ namespace Catch { } }; - ReusableStringStream::ReusableStringStream() - : m_index( Singleton::getMutable().add() ), - m_oss( Singleton::getMutable().m_streams[m_index].get() ) - {} + ReusableStringStream::ReusableStringStream() { + std::tie( m_index, m_oss ) = + Singleton::getMutable().add(); + } ReusableStringStream::~ReusableStringStream() { static_cast( m_oss )->str("");