From 58137e50e4cca0f0bc80647c038db8155ceffdd7 Mon Sep 17 00:00:00 2001 From: Jeremy Rifkin <51220084+jeremy-rifkin@users.noreply.github.com> Date: Sat, 18 Jan 2025 14:33:24 -0600 Subject: [PATCH] Better handle the global lock to avoid static init issues --- .../internal/catch_assertion_handler.cpp | 18 +++++++++--------- src/catch2/internal/catch_global_lock.cpp | 5 ++++- src/catch2/internal/catch_global_lock.hpp | 6 +++--- .../internal/catch_reusable_string_stream.cpp | 4 ++-- src/catch2/internal/catch_run_context.cpp | 8 ++++---- .../reporters/catch_reporter_console.cpp | 8 ++++---- 6 files changed, 26 insertions(+), 23 deletions(-) diff --git a/src/catch2/internal/catch_assertion_handler.cpp b/src/catch2/internal/catch_assertion_handler.cpp index 36dc72f1..e6829b46 100644 --- a/src/catch2/internal/catch_assertion_handler.cpp +++ b/src/catch2/internal/catch_assertion_handler.cpp @@ -25,23 +25,23 @@ namespace Catch { : m_assertionInfo{ macroName, lineInfo, capturedExpression, resultDisposition }, m_resultCapture( getResultCapture() ) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); m_resultCapture.notifyAssertionStarted( m_assertionInfo ); } AssertionHandler::~AssertionHandler() { - auto lock = get_global_lock(); + auto lock = take_global_lock(); if ( !m_completed ) { m_resultCapture.handleIncomplete( m_assertionInfo ); } } void AssertionHandler::handleExpr( ITransientExpression const& expr ) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); m_resultCapture.handleExpr( m_assertionInfo, expr, m_reaction ); } void AssertionHandler::handleMessage(ResultWas::OfType resultType, std::string&& message) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); m_resultCapture.handleMessage( m_assertionInfo, resultType, CATCH_MOVE(message), m_reaction ); } @@ -68,26 +68,26 @@ namespace Catch { } void AssertionHandler::handleUnexpectedInflightException() { - auto lock = get_global_lock(); + auto lock = take_global_lock(); m_resultCapture.handleUnexpectedInflightException( m_assertionInfo, Catch::translateActiveException(), m_reaction ); } void AssertionHandler::handleExceptionThrownAsExpected() { - auto lock = get_global_lock(); + auto lock = take_global_lock(); m_resultCapture.handleNonExpr(m_assertionInfo, ResultWas::Ok, m_reaction); } void AssertionHandler::handleExceptionNotThrownAsExpected() { - auto lock = get_global_lock(); + auto lock = take_global_lock(); m_resultCapture.handleNonExpr(m_assertionInfo, ResultWas::Ok, m_reaction); } void AssertionHandler::handleUnexpectedExceptionNotThrown() { - auto lock = get_global_lock(); + auto lock = take_global_lock(); m_resultCapture.handleUnexpectedExceptionNotThrown( m_assertionInfo, m_reaction ); } void AssertionHandler::handleThrowingCallSkipped() { - auto lock = get_global_lock(); + auto lock = take_global_lock(); m_resultCapture.handleNonExpr(m_assertionInfo, ResultWas::Ok, m_reaction); } diff --git a/src/catch2/internal/catch_global_lock.cpp b/src/catch2/internal/catch_global_lock.cpp index 1ec9eab5..8a178f2b 100644 --- a/src/catch2/internal/catch_global_lock.cpp +++ b/src/catch2/internal/catch_global_lock.cpp @@ -13,7 +13,10 @@ CATCH_INTERNAL_SUPPRESS_GLOBALS_WARNINGS namespace Catch { - std::recursive_mutex global_lock; + std::recursive_mutex& get_global_lock() { + static std::recursive_mutex global_lock; + return global_lock; + } } // namespace Catch diff --git a/src/catch2/internal/catch_global_lock.hpp b/src/catch2/internal/catch_global_lock.hpp index 77971056..2d622498 100644 --- a/src/catch2/internal/catch_global_lock.hpp +++ b/src/catch2/internal/catch_global_lock.hpp @@ -13,10 +13,10 @@ namespace Catch { - extern std::recursive_mutex global_lock; + std::recursive_mutex& get_global_lock(); - inline auto get_global_lock() { - return std::unique_lock(global_lock); + inline auto take_global_lock() { + return std::unique_lock(get_global_lock()); } } // namespace Catch diff --git a/src/catch2/internal/catch_reusable_string_stream.cpp b/src/catch2/internal/catch_reusable_string_stream.cpp index a32b83e9..ce02e33c 100644 --- a/src/catch2/internal/catch_reusable_string_stream.cpp +++ b/src/catch2/internal/catch_reusable_string_stream.cpp @@ -45,13 +45,13 @@ namespace Catch { // instead of poking around StringStreams and Singleton. ReusableStringStream::ReusableStringStream() { - auto lock = get_global_lock(); + auto lock = take_global_lock(); m_index = Singleton::getMutable().add(); m_oss = Singleton::getMutable().m_streams[m_index].get(); } ReusableStringStream::~ReusableStringStream() { - auto lock = get_global_lock(); + auto lock = take_global_lock(); static_cast( m_oss )->str(""); m_oss->clear(); Singleton::getMutable().release( m_index ); diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp index b8a321f6..cece2d94 100644 --- a/src/catch2/internal/catch_run_context.cpp +++ b/src/catch2/internal/catch_run_context.cpp @@ -422,22 +422,22 @@ namespace Catch { // Catch benchmark macros call these functions. Since catch internals are not thread-safe locking is needed. void RunContext::benchmarkPreparing( StringRef name ) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); auto _ = scopedDeactivate( *m_outputRedirect ); m_reporter->benchmarkPreparing( name ); } void RunContext::benchmarkStarting( BenchmarkInfo const& info ) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); auto _ = scopedDeactivate( *m_outputRedirect ); m_reporter->benchmarkStarting( info ); } void RunContext::benchmarkEnded( BenchmarkStats<> const& stats ) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); auto _ = scopedDeactivate( *m_outputRedirect ); m_reporter->benchmarkEnded( stats ); } void RunContext::benchmarkFailed( StringRef error ) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); auto _ = scopedDeactivate( *m_outputRedirect ); m_reporter->benchmarkFailed( error ); } diff --git a/src/catch2/reporters/catch_reporter_console.cpp b/src/catch2/reporters/catch_reporter_console.cpp index e7bc6ed9..94abe796 100644 --- a/src/catch2/reporters/catch_reporter_console.cpp +++ b/src/catch2/reporters/catch_reporter_console.cpp @@ -466,7 +466,7 @@ void ConsoleReporter::sectionEnded(SectionStats const& _sectionStats) { // Catch benchmark macros call these functions. Since catch internals are not thread-safe locking is needed. void ConsoleReporter::benchmarkPreparing( StringRef name ) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); lazyPrintWithoutClosingBenchmarkTable(); auto nameCol = TextFlow::Column( static_cast( name ) ) @@ -484,7 +484,7 @@ void ConsoleReporter::benchmarkPreparing( StringRef name ) { } void ConsoleReporter::benchmarkStarting(BenchmarkInfo const& info) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); (*m_tablePrinter) << info.samples << ColumnBreak() << info.iterations << ColumnBreak(); if ( !m_config->benchmarkNoAnalysis() ) { @@ -494,7 +494,7 @@ void ConsoleReporter::benchmarkStarting(BenchmarkInfo const& info) { ( *m_tablePrinter ) << OutputFlush{}; } void ConsoleReporter::benchmarkEnded(BenchmarkStats<> const& stats) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); if (m_config->benchmarkNoAnalysis()) { (*m_tablePrinter) << Duration(stats.mean.point.count()) << ColumnBreak(); @@ -512,7 +512,7 @@ void ConsoleReporter::benchmarkEnded(BenchmarkStats<> const& stats) { } void ConsoleReporter::benchmarkFailed( StringRef error ) { - auto lock = get_global_lock(); + auto lock = take_global_lock(); auto guard = m_colour->guardColour( Colour::Red ).engage( m_stream ); (*m_tablePrinter) << "Benchmark failed (" << error << ')'