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).
This commit is contained in:
Duncan Horn
2025-09-25 04:31:49 -07:00
committed by GitHub
parent b626e4c7ae
commit f7e7fa0983

View File

@@ -12,6 +12,7 @@
#include <cstdio> #include <cstdio>
#include <sstream> #include <sstream>
#include <tuple>
#include <vector> #include <vector>
namespace Catch { namespace Catch {
@@ -23,16 +24,16 @@ namespace Catch {
std::ostringstream m_referenceStream; // Used for copy state/ flags from std::ostringstream m_referenceStream; // Used for copy state/ flags from
Detail::Mutex m_mutex; Detail::Mutex m_mutex;
auto add() -> std::size_t { auto add() -> std::pair<std::size_t, std::ostringstream*> {
Detail::LockGuard _( m_mutex ); Detail::LockGuard _( m_mutex );
if( m_unused.empty() ) { if( m_unused.empty() ) {
m_streams.push_back( Detail::make_unique<std::ostringstream>() ); m_streams.push_back( Detail::make_unique<std::ostringstream>() );
return m_streams.size()-1; return { m_streams.size()-1, m_streams.back().get() };
} }
else { else {
auto index = m_unused.back(); auto index = m_unused.back();
m_unused.pop_back(); m_unused.pop_back();
return index; return { index, m_streams[index].get() };
} }
} }
@@ -46,10 +47,10 @@ namespace Catch {
} }
}; };
ReusableStringStream::ReusableStringStream() ReusableStringStream::ReusableStringStream() {
: m_index( Singleton<StringStreams>::getMutable().add() ), std::tie( m_index, m_oss ) =
m_oss( Singleton<StringStreams>::getMutable().m_streams[m_index].get() ) Singleton<StringStreams>::getMutable().add();
{} }
ReusableStringStream::~ReusableStringStream() { ReusableStringStream::~ReusableStringStream() {
static_cast<std::ostringstream*>( m_oss )->str(""); static_cast<std::ostringstream*>( m_oss )->str("");