diff --git a/CMakeLists.txt b/CMakeLists.txt index 9d765212..97f8c227 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,6 +20,7 @@ cmake_dependent_option(CATCH_BUILD_TESTING "Build the SelfTest project" ON "CATC cmake_dependent_option(CATCH_BUILD_EXAMPLES "Build code examples" OFF "CATCH_DEVELOPMENT_BUILD" OFF) cmake_dependent_option(CATCH_BUILD_EXTRA_TESTS "Build extra tests" OFF "CATCH_DEVELOPMENT_BUILD" OFF) cmake_dependent_option(CATCH_BUILD_FUZZERS "Build fuzzers" OFF "CATCH_DEVELOPMENT_BUILD" OFF) +cmake_dependent_option(CATCH_BUILD_BENCHMARKS "Build benchmarks" OFF "CATCH_DEVELOPMENT_BUILD" OFF) cmake_dependent_option(CATCH_ENABLE_COVERAGE "Generate coverage for codecov.io" OFF "CATCH_DEVELOPMENT_BUILD" OFF) cmake_dependent_option(CATCH_ENABLE_WERROR "Enables Werror during build" ON "CATCH_DEVELOPMENT_BUILD" OFF) cmake_dependent_option(CATCH_BUILD_SURROGATES "Enable generating and building surrogate TUs for the main headers" OFF "CATCH_DEVELOPMENT_BUILD" OFF) @@ -104,6 +105,11 @@ if(CATCH_BUILD_FUZZERS) add_subdirectory(fuzzing) endif() +if(CATCH_BUILD_BENCHMARKS) + set(CMAKE_FOLDER "benchmarks") + add_subdirectory(benchmarks) +endif() + if (CATCH_DEVELOPMENT_BUILD) add_warnings_to_targets("${CATCH_WARNING_TARGETS}") endif() @@ -156,7 +162,7 @@ if (NOT_SUBPROJECT) DESTINATION ${CATCH_CMAKE_CONFIG_DESTINATION} ) - + # Install debugger helpers install( FILES diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt new file mode 100644 index 00000000..66c2f51d --- /dev/null +++ b/benchmarks/CMakeLists.txt @@ -0,0 +1,3 @@ +add_executable(benchmarks catch_benchmarks.cpp) +target_link_libraries(benchmarks PRIVATE Catch2WithMain) +target_compile_features(benchmarks PUBLIC cxx_std_17) diff --git a/benchmarks/catch_benchmarks.cpp b/benchmarks/catch_benchmarks.cpp new file mode 100644 index 00000000..8b0f8479 --- /dev/null +++ b/benchmarks/catch_benchmarks.cpp @@ -0,0 +1,25 @@ +#include +#include + +#include + +std::recursive_mutex global_lock; + +int no_lock() { + return 2; +} + +int take_lock() { + std::unique_lock lock(global_lock); + return 2; +} + +TEST_CASE("std::recursive_mutex overhead benchmark", "[benchmark][mutex]") { + BENCHMARK("no lock") { + return no_lock(); + }; + + BENCHMARK("with std::recursive_mutex") { + return take_lock(); + }; +} diff --git a/docs/limitations.md b/docs/limitations.md index f5f60ba8..f433bf35 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -58,23 +58,9 @@ again. This section outlines some missing features, what is their status and their possible workarounds. ### Thread safe assertions -Catch2's assertion macros are not thread safe. This does not mean that -you cannot use threads inside Catch's test, but that only single thread -can interact with Catch's assertions and other macros. +Catch2's assertion macros and logging macros are thread safe. -This means that this is ok -```cpp - std::vector threads; - std::atomic cnt{ 0 }; - for (int i = 0; i < 4; ++i) { - threads.emplace_back([&]() { - ++cnt; ++cnt; ++cnt; ++cnt; - }); - } - for (auto& t : threads) { t.join(); } - REQUIRE(cnt == 16); -``` -because only one thread passes the `REQUIRE` macro and this is not +This is ok however it was previously not ok for Catch2 3.8.0 and earlier: ```cpp std::vector threads; std::atomic cnt{ 0 }; @@ -88,8 +74,6 @@ because only one thread passes the `REQUIRE` macro and this is not REQUIRE(cnt == 16); ``` -We currently do not plan to support thread-safe assertions. - ### Process isolation in a test Catch does not support running tests in isolated (forked) processes. While this might in the future, the fact that Windows does not support forking and only allows full-on process creation and the desire to keep code as similar as possible across platforms, mean that this is likely to take significant development time, that is not currently available. diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c40de040..2a61b898 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -182,6 +182,7 @@ set(IMPL_SOURCES ${SOURCES_DIR}/internal/catch_fatal_condition_handler.cpp ${SOURCES_DIR}/internal/catch_floating_point_helpers.cpp ${SOURCES_DIR}/internal/catch_getenv.cpp + ${SOURCES_DIR}/internal/catch_global_lock.cpp ${SOURCES_DIR}/internal/catch_istream.cpp ${SOURCES_DIR}/internal/catch_jsonwriter.cpp ${SOURCES_DIR}/internal/catch_lazy_expr.cpp diff --git a/src/catch2/internal/catch_assertion_handler.cpp b/src/catch2/internal/catch_assertion_handler.cpp index 9a28e79c..36dc72f1 100644 --- a/src/catch2/internal/catch_assertion_handler.cpp +++ b/src/catch2/internal/catch_assertion_handler.cpp @@ -10,9 +10,12 @@ #include #include #include +#include #include namespace Catch { + // The AssertionHandler API and handleExceptionMatchExpr are used by assertion macros. Everything here must be + // locked as catch internals are not thread-safe. AssertionHandler::AssertionHandler ( StringRef macroName, @@ -22,13 +25,23 @@ namespace Catch { : m_assertionInfo{ macroName, lineInfo, capturedExpression, resultDisposition }, m_resultCapture( getResultCapture() ) { + auto lock = get_global_lock(); m_resultCapture.notifyAssertionStarted( m_assertionInfo ); } + AssertionHandler::~AssertionHandler() { + auto lock = get_global_lock(); + if ( !m_completed ) { + m_resultCapture.handleIncomplete( m_assertionInfo ); + } + } + void AssertionHandler::handleExpr( ITransientExpression const& expr ) { + auto lock = get_global_lock(); m_resultCapture.handleExpr( m_assertionInfo, expr, m_reaction ); } void AssertionHandler::handleMessage(ResultWas::OfType resultType, std::string&& message) { + auto lock = get_global_lock(); m_resultCapture.handleMessage( m_assertionInfo, resultType, CATCH_MOVE(message), m_reaction ); } @@ -55,21 +68,26 @@ namespace Catch { } void AssertionHandler::handleUnexpectedInflightException() { + auto lock = get_global_lock(); m_resultCapture.handleUnexpectedInflightException( m_assertionInfo, Catch::translateActiveException(), m_reaction ); } void AssertionHandler::handleExceptionThrownAsExpected() { + auto lock = get_global_lock(); m_resultCapture.handleNonExpr(m_assertionInfo, ResultWas::Ok, m_reaction); } void AssertionHandler::handleExceptionNotThrownAsExpected() { + auto lock = get_global_lock(); m_resultCapture.handleNonExpr(m_assertionInfo, ResultWas::Ok, m_reaction); } void AssertionHandler::handleUnexpectedExceptionNotThrown() { + auto lock = get_global_lock(); m_resultCapture.handleUnexpectedExceptionNotThrown( m_assertionInfo, m_reaction ); } void AssertionHandler::handleThrowingCallSkipped() { + auto lock = get_global_lock(); m_resultCapture.handleNonExpr(m_assertionInfo, ResultWas::Ok, m_reaction); } diff --git a/src/catch2/internal/catch_assertion_handler.hpp b/src/catch2/internal/catch_assertion_handler.hpp index c71c6898..083f9299 100644 --- a/src/catch2/internal/catch_assertion_handler.hpp +++ b/src/catch2/internal/catch_assertion_handler.hpp @@ -34,12 +34,7 @@ namespace Catch { SourceLineInfo const& lineInfo, StringRef capturedExpression, ResultDisposition::Flags resultDisposition ); - ~AssertionHandler() { - if ( !m_completed ) { - m_resultCapture.handleIncomplete( m_assertionInfo ); - } - } - + ~AssertionHandler(); template constexpr void handleExpr( ExprLhs const& expr ) { diff --git a/src/catch2/internal/catch_global_lock.cpp b/src/catch2/internal/catch_global_lock.cpp new file mode 100644 index 00000000..424ead0e --- /dev/null +++ b/src/catch2/internal/catch_global_lock.cpp @@ -0,0 +1,14 @@ + +// Copyright Catch2 Authors +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// SPDX-License-Identifier: BSL-1.0 +#include + +namespace Catch { + + std::recursive_mutex global_lock; + +} // namespace Catch diff --git a/src/catch2/internal/catch_global_lock.hpp b/src/catch2/internal/catch_global_lock.hpp new file mode 100644 index 00000000..0408f33f --- /dev/null +++ b/src/catch2/internal/catch_global_lock.hpp @@ -0,0 +1,23 @@ +// Copyright Catch2 Authors +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// SPDX-License-Identifier: BSL-1.0 + +#ifndef CATCH_GLOBAL_LOCK_HPP_INCLUDED +#define CATCH_GLOBAL_LOCK_HPP_INCLUDED + +#include + +namespace Catch { + + extern std::recursive_mutex global_lock; + + inline auto get_global_lock() { + return std::unique_lock(global_lock); + } + +} // namespace Catch + +#endif // CATCH_GLOBAL_LOCK_HPP_INCLUDED diff --git a/src/catch2/internal/catch_reusable_string_stream.cpp b/src/catch2/internal/catch_reusable_string_stream.cpp index 33eafde4..a32b83e9 100644 --- a/src/catch2/internal/catch_reusable_string_stream.cpp +++ b/src/catch2/internal/catch_reusable_string_stream.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -39,12 +40,18 @@ namespace Catch { } }; - ReusableStringStream::ReusableStringStream() - : m_index( Singleton::getMutable().add() ), - m_oss( Singleton::getMutable().m_streams[m_index].get() ) - {} + // Catch message macros create MessageStreams which hold ReusableStringStream. Since catch internals are not + // thread-safe locking is needed and it's easiest to lock at the ReusableStringStream construct/destruct level + // instead of poking around StringStreams and Singleton. + + ReusableStringStream::ReusableStringStream() { + auto lock = get_global_lock(); + m_index = Singleton::getMutable().add(); + m_oss = Singleton::getMutable().m_streams[m_index].get(); + } ReusableStringStream::~ReusableStringStream() { + auto lock = get_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 2a102fbe..b8a321f6 100644 --- a/src/catch2/internal/catch_run_context.cpp +++ b/src/catch2/internal/catch_run_context.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -418,19 +419,25 @@ namespace Catch { m_unfinishedSections.push_back(CATCH_MOVE(endInfo)); } + // 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 _ = scopedDeactivate( *m_outputRedirect ); m_reporter->benchmarkPreparing( name ); } void RunContext::benchmarkStarting( BenchmarkInfo const& info ) { + auto lock = get_global_lock(); auto _ = scopedDeactivate( *m_outputRedirect ); m_reporter->benchmarkStarting( info ); } void RunContext::benchmarkEnded( BenchmarkStats<> const& stats ) { + auto lock = get_global_lock(); auto _ = scopedDeactivate( *m_outputRedirect ); m_reporter->benchmarkEnded( stats ); } void RunContext::benchmarkFailed( StringRef error ) { + auto lock = get_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 9529f39a..e7bc6ed9 100644 --- a/src/catch2/reporters/catch_reporter_console.cpp +++ b/src/catch2/reporters/catch_reporter_console.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -462,7 +463,10 @@ void ConsoleReporter::sectionEnded(SectionStats const& _sectionStats) { StreamingReporterBase::sectionEnded(_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(); lazyPrintWithoutClosingBenchmarkTable(); auto nameCol = TextFlow::Column( static_cast( name ) ) @@ -480,6 +484,7 @@ void ConsoleReporter::benchmarkPreparing( StringRef name ) { } void ConsoleReporter::benchmarkStarting(BenchmarkInfo const& info) { + auto lock = get_global_lock(); (*m_tablePrinter) << info.samples << ColumnBreak() << info.iterations << ColumnBreak(); if ( !m_config->benchmarkNoAnalysis() ) { @@ -489,6 +494,7 @@ void ConsoleReporter::benchmarkStarting(BenchmarkInfo const& info) { ( *m_tablePrinter ) << OutputFlush{}; } void ConsoleReporter::benchmarkEnded(BenchmarkStats<> const& stats) { + auto lock = get_global_lock(); if (m_config->benchmarkNoAnalysis()) { (*m_tablePrinter) << Duration(stats.mean.point.count()) << ColumnBreak(); @@ -506,6 +512,7 @@ void ConsoleReporter::benchmarkEnded(BenchmarkStats<> const& stats) { } void ConsoleReporter::benchmarkFailed( StringRef error ) { + auto lock = get_global_lock(); auto guard = m_colour->guardColour( Colour::Red ).engage( m_stream ); (*m_tablePrinter) << "Benchmark failed (" << error << ')' diff --git a/tests/ExtraTests/CMakeLists.txt b/tests/ExtraTests/CMakeLists.txt index 3c38d675..baf7b913 100644 --- a/tests/ExtraTests/CMakeLists.txt +++ b/tests/ExtraTests/CMakeLists.txt @@ -479,6 +479,25 @@ set_tests_properties( LABELS "uses-signals" ) +add_executable(Multithreading ${TESTS_DIR}/X37-Multithreading.cpp) +target_link_libraries(Multithreading PRIVATE Catch2::Catch2WithMain) +add_test( + NAME Reporters::Multithreading + COMMAND ${CMAKE_COMMAND} -E env $ +) +set_tests_properties( + Reporters::Multithreading + PROPERTIES + PASS_REGULAR_EXPRESSION "passed" + FAIL_REGULAR_EXPRESSION "ThreadSanitizer" +) +if (NOT WIN32) + target_compile_options( Reporters::Multithreading + PUBLIC + $<$,$,$>:-fsanitize=thread> + ) +endif() + add_executable(AssertionStartingEventGoesBeforeAssertionIsEvaluated X20-AssertionStartingEventGoesBeforeAssertionIsEvaluated.cpp ) diff --git a/tests/ExtraTests/X37-Multithreading.cpp b/tests/ExtraTests/X37-Multithreading.cpp new file mode 100644 index 00000000..30a238ab --- /dev/null +++ b/tests/ExtraTests/X37-Multithreading.cpp @@ -0,0 +1,34 @@ +// Copyright Catch2 Authors +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// SPDX-License-Identifier: BSL-1.0 + +#include +#include +#include + +#include +#include + +TEST_CASE( "ThreadAssertionTest", + "[Multithreading]" ) { + SECTION( "Basic" ) { + std::jthread a([] (const std::stop_token& token) { + while (!token.stop_requested()) { + FAIL_CHECK(false); + CHECK(true); + } + }); + std::jthread b([] (const std::stop_token& token) { + while (!token.stop_requested()) { + FAIL_CHECK(false); + CHECK(true); + } + }); + std::this_thread::sleep_for( std::chrono::milliseconds( 1'000 ) ); + a.get_stop_source().request_stop(); + b.get_stop_source().request_stop(); + } +}