diff --git a/docs/thread-safety.md b/docs/thread-safety.md
index c3f610f0..3d48eb0b 100644
--- a/docs/thread-safety.md
+++ b/docs/thread-safety.md
@@ -2,7 +2,9 @@
# Thread safety in Catch2
**Contents**
-[Using assertion macros from multiple threads](#using-assertion-macros-from-multiple-threads)
+[Using assertion macros from spawned threads](#using-assertion-macros-from-spawned-threads)
+[Assertion-like message macros and spawned threads](#assertion-like-message-macros-and-spawned-threads)
+[Message macros and spawned threads](#message-macros-and-spawned-threads)
[examples](#examples)
[`STATIC_REQUIRE` and `STATIC_CHECK`](#static_require-and-static_check)
[Fatal errors and multiple threads](#fatal-errors-and-multiple-threads)
@@ -10,17 +12,18 @@
> Thread safe assertions were introduced in Catch2 3.9.0
-Thread safety in Catch2 is currently limited to all the assertion macros.
-Interacting with benchmark macros, message macros (e.g. `INFO` or `CAPTURE`),
-sections macros, generator macros, or test case macros is not thread-safe.
-The message macros are likely to be made thread-safe in the future, but
-the way sections define test runs is incompatible with user being able
-to spawn threads arbitrarily, thus that limitation is here to stay.
+Thread safety in Catch2 is currently limited to all the assertion macros,
+and to message or message-adjacent macros (e.g. `INFO` or `WARN`).
+
+Interacting with benchmark macros, sections macros, generator macros, or
+test case macros is not thread-safe. The way sections define paths through
+the test is incompatible with user spawning threads arbitrarily, so this
+limitation is here to stay.
**Important: thread safety in Catch2 is [opt-in](configuration.md#experimental-thread-safety)**
-## Using assertion macros from multiple threads
+## Using assertion macros from spawned threads
The full set of Catch2's runtime assertion macros is thread-safe. However,
it is important to keep in mind that their semantics might not support
@@ -30,7 +33,7 @@ Specifically, the `REQUIRE` family of assertion macros have semantics
of stopping the test execution on failure. This is done by throwing
an exception, but since the user-spawned thread will not have the test-level
try-catch block ready to catch the test failure exception, failing a
-`REQUIRE` assertion inside this thread will terminate the process.
+`REQUIRE` assertion inside user-spawned thread will terminate the process.
The `CHECK` family of assertions does not have this issue, because it
does not try to stop the test execution.
@@ -38,16 +41,32 @@ does not try to stop the test execution.
Note that `CHECKED_IF` and `CHECKED_ELSE` are also thread safe (internally
they are assertion macro + an if).
-**`SKIP()`, `FAIL()`, `SUCCEED()` are not assertion macros, and are not
-thread-safe.**
+
+## Assertion-like message macros and spawned threads
+
+Similarly to assertion macros, not all assertion-like message macros can
+be used from spawned thread.
+
+`SKIP` and `FAIL` macros stop the test execution. Just like with `REQUIRE`,
+this means that they cannot be used inside user-spawned threads. `SUCCEED`,
+`FAIL_CHECK` and `WARN` do not attempt to stop the test execution and
+thus can be used from any thread.
+
+
+## Message macros and spawned threads
+
+Macros that add extra messages to following assertion, such as `INFO`
+or `CAPTURE`, are all thread safe and can be used in any thread. Note
+that these messages are per-thread, and thus `INFO` inside a user-spawned
+thread will not be seen by the main thread, and vice versa.
## examples
-### `REQUIRE` from main thread, `CHECK` from spawned threads
+### `REQUIRE` from the main thread, `CHECK` from spawned threads
```cpp
-TEST_CASE( "Failed REQUIRE in main thread is fine" ) {
+TEST_CASE( "Failed REQUIRE in the main thread is fine" ) {
std::vector threads;
for ( size_t t = 0; t < 16; ++t) {
threads.emplace_back( []() {
@@ -85,7 +104,7 @@ TEST_CASE( "Successful REQUIRE in spawned thread is fine" ) {
This will also work as expected, because the `REQUIRE` is successful.
```cpp
-TEST_CASE( "Failed REQUIRE in spawned thread is fine" ) {
+TEST_CASE( "Failed REQUIRE in spawned thread kills the process" ) {
std::vector threads;
for ( size_t t = 0; t < 16; ++t) {
threads.emplace_back( []() {
@@ -99,12 +118,88 @@ TEST_CASE( "Failed REQUIRE in spawned thread is fine" ) {
This will fail catastrophically and terminate the process.
+### INFO across threads
+
+```cpp
+TEST_CASE( "messages don't cross threads" ) {
+ std::jthread t1( [&]() {
+ for ( size_t i = 0; i < 100; ++i ) {
+ INFO( "spawned thread #1" );
+ CHECK( 1 == 1 );
+ }
+ } );
+
+ std::thread t2( [&]() {
+ for (size_t i = 0; i < 100; ++i) {
+ UNSCOPED_INFO( "spawned thread #2" );
+ }
+ } );
+
+ for (size_t i = 0; i < 100; ++i) {
+ CHECK( 1 == 2 );
+ }
+}
+```
+None of the failed checks will show the "spawned thread #1" message, as
+that message is for the `t1` thread. If the reporter shows passing
+assertions (e.g. due to the tests being run with `-s`), you will see the
+"spawned thread #1" message alongside the passing `CHECK( 1 == 1 )` assertion.
+
+The message "spawned thread #2" will never be shown, because there are no
+assertions in `t2`.
+
+
+### FAIL/SKIP from the main thread
+
+```cpp
+TEST_CASE( "FAIL in the main thread is fine" ) {
+ std::vector threads;
+ for ( size_t t = 0; t < 16; ++t) {
+ threads.emplace_back( []() {
+ for (size_t i = 0; i < 10; ++i) {
+ CHECK( true );
+ CHECK( false );
+ }
+ } );
+ }
+
+ FAIL();
+}
+```
+
+This will work as expected, that is, the process will finish running
+normally, the test case will fail and there will be 321 total assertions,
+160 passing and 161 failing (`FAIL` counts as failed assertion).
+
+However, when the main thread hits `FAIL`, it will wait for the other
+threads to finish due to `std::jthread`'s destructor joining the spawned
+thread. Due to this, using `SKIP` is not recommended once more threads
+are spawned; while the main thread will bail from the test execution,
+the spawned threads will keep running and may fail the test case.
+
+
+### FAIL/SKIP from spawned threads
+
+```cpp
+TEST_CASE( "FAIL/SKIP in spawned thread kills the process" ) {
+ std::vector threads;
+ for ( size_t t = 0; t < 16; ++t) {
+ threads.emplace_back( []() {
+ for (size_t i = 0; i < 10'000; ++i) {
+ FAIL();
+ }
+ } );
+ }
+}
+```
+As with failing `REQUIRE`, both `FAIL` and `SKIP` in spawned threads
+terminate the process.
+
+
## `STATIC_REQUIRE` and `STATIC_CHECK`
-None of `STATIC_REQUIRE`, `STATIC_REQUIRE_FALSE`, `STATIC_CHECK`, and
-`STATIC_CHECK_FALSE` are currently thread safe. This might be surprising
-given that they are a compile-time checks, but they also rely on the
-message macros to register the result with reporter at runtime.
+All of `STATIC_REQUIRE`, `STATIC_REQUIRE_FALSE`, `STATIC_CHECK`, and
+`STATIC_CHECK_FALSE` are thread safe in the delayed evaluation configuration.
## Fatal errors and multiple threads
diff --git a/src/catch2/catch_message.cpp b/src/catch2/catch_message.cpp
index d3914a80..9cb0167f 100644
--- a/src/catch2/catch_message.cpp
+++ b/src/catch2/catch_message.cpp
@@ -101,8 +101,9 @@ namespace Catch {
}
Capturer::~Capturer() {
assert( m_captured == m_messages.size() );
- for ( size_t i = 0; i < m_captured; ++i )
+ for ( size_t i = 0; i < m_captured; ++i ) {
m_resultCapture.popScopedMessage( m_messages[i] );
+ }
}
void Capturer::captureValue( size_t index, std::string const& value ) {
diff --git a/src/catch2/internal/catch_message_info.cpp b/src/catch2/internal/catch_message_info.cpp
index e0e9dc7e..bc887abf 100644
--- a/src/catch2/internal/catch_message_info.cpp
+++ b/src/catch2/internal/catch_message_info.cpp
@@ -10,16 +10,17 @@
namespace Catch {
- MessageInfo::MessageInfo( StringRef _macroName,
- SourceLineInfo const& _lineInfo,
- ResultWas::OfType _type )
+ MessageInfo::MessageInfo( StringRef _macroName,
+ SourceLineInfo const& _lineInfo,
+ ResultWas::OfType _type )
: macroName( _macroName ),
lineInfo( _lineInfo ),
type( _type ),
sequence( ++globalCount )
{}
- // This may need protecting if threading support is added
- unsigned int MessageInfo::globalCount = 0;
+ // Messages are owned by their individual threads, so the counter should be thread-local as well.
+ // Alternative consideration: atomic, so threads don't share IDs and things are easier to debug.
+ thread_local unsigned int MessageInfo::globalCount = 0;
} // end namespace Catch
diff --git a/src/catch2/internal/catch_message_info.hpp b/src/catch2/internal/catch_message_info.hpp
index ded098f5..bd686d6d 100644
--- a/src/catch2/internal/catch_message_info.hpp
+++ b/src/catch2/internal/catch_message_info.hpp
@@ -37,7 +37,7 @@ namespace Catch {
return sequence < other.sequence;
}
private:
- static unsigned int globalCount;
+ static thread_local unsigned int globalCount;
};
} // end namespace Catch
diff --git a/src/catch2/internal/catch_reusable_string_stream.cpp b/src/catch2/internal/catch_reusable_string_stream.cpp
index 33eafde4..a2dd5a63 100644
--- a/src/catch2/internal/catch_reusable_string_stream.cpp
+++ b/src/catch2/internal/catch_reusable_string_stream.cpp
@@ -7,6 +7,7 @@
// SPDX-License-Identifier: BSL-1.0
#include
#include
+#include
#include
#include
@@ -20,8 +21,10 @@ namespace Catch {
std::vector> m_streams;
std::vector m_unused;
std::ostringstream m_referenceStream; // Used for copy state/ flags from
+ Detail::Mutex m_mutex;
auto add() -> std::size_t {
+ Detail::LockGuard _( m_mutex );
if( m_unused.empty() ) {
m_streams.push_back( Detail::make_unique() );
return m_streams.size()-1;
@@ -33,9 +36,13 @@ namespace Catch {
}
}
- void release( std::size_t index ) {
- m_streams[index]->copyfmt( m_referenceStream ); // Restore initial flags and other state
- m_unused.push_back(index);
+ void release( std::size_t index, std::ostream* originalPtr ) {
+ assert( originalPtr );
+ originalPtr->copyfmt( m_referenceStream ); // Restore initial flags and other state
+
+ Detail::LockGuard _( m_mutex );
+ assert( originalPtr == m_streams[index].get() && "Mismatch between release index and stream ptr" );
+ m_unused.push_back( index );
}
};
@@ -47,7 +54,7 @@ namespace Catch {
ReusableStringStream::~ReusableStringStream() {
static_cast( m_oss )->str("");
m_oss->clear();
- Singleton::getMutable().release( m_index );
+ Singleton::getMutable().release( m_index, m_oss );
}
std::string ReusableStringStream::str() const {
diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp
index 15e25322..d1cec82c 100644
--- a/src/catch2/internal/catch_run_context.cpp
+++ b/src/catch2/internal/catch_run_context.cpp
@@ -172,9 +172,6 @@ namespace Catch {
// This also implies that messages are owned by their respective
// threads, and should not be shared across different threads.
//
- // For simplicity, we disallow messages in multi-threaded contexts,
- // but in the future we can enable them under this logic.
- //
// This implies that various pieces of metadata referring to last
// assertion result/source location/message handling, etc
// should also be thread local. For now we just use naked globals
@@ -183,15 +180,27 @@ namespace Catch {
// This is used for the "if" part of CHECKED_IF/CHECKED_ELSE
static thread_local bool g_lastAssertionPassed = false;
- // Should we clear message scopes before sending off the messages to
- // reporter? Set in `assertionPassedFastPath` to avoid doing the full
- // clear there for performance reasons.
- static thread_local bool g_clearMessageScopes = false;
+
// This is the source location for last encountered macro. It is
// used to provide the users with more precise location of error
// when an unexpected exception/fatal error happens.
static thread_local SourceLineInfo g_lastKnownLineInfo("DummyLocation", static_cast(-1));
- }
+
+ // Should we clear message scopes before sending off the messages to
+ // reporter? Set in `assertionPassedFastPath` to avoid doing the full
+ // clear there for performance reasons.
+ static thread_local bool g_clearMessageScopes = false;
+
+ CATCH_INTERNAL_START_WARNINGS_SUPPRESSION
+ CATCH_INTERNAL_SUPPRESS_GLOBALS_WARNINGS
+ // Actual messages to be provided to the reporter
+ static thread_local std::vector g_messages;
+
+ // Owners for the UNSCOPED_X information macro
+ static thread_local std::vector g_messageScopes;
+ CATCH_INTERNAL_STOP_WARNINGS_SUPPRESSION
+
+ } // namespace Detail
RunContext::RunContext(IConfig const* _config, IEventListenerPtr&& reporter)
: m_runInfo(_config->name()),
@@ -327,20 +336,21 @@ namespace Catch {
Detail::g_lastAssertionPassed = true;
}
+ if ( Detail::g_clearMessageScopes ) {
+ Detail::g_messageScopes.clear();
+ Detail::g_clearMessageScopes = false;
+ }
+
// From here, we are touching shared state and need mutex.
Detail::LockGuard lock( m_assertionMutex );
{
- if ( Detail::g_clearMessageScopes ) {
- m_messageScopes.clear();
- Detail::g_clearMessageScopes = false;
- }
auto _ = scopedDeactivate( *m_outputRedirect );
updateTotalsFromAtomics();
- m_reporter->assertionEnded( AssertionStats( result, m_messages, m_totals ) );
+ m_reporter->assertionEnded( AssertionStats( result, Detail::g_messages, m_totals ) );
}
if ( result.getResultType() != ResultWas::Warning ) {
- m_messageScopes.clear();
+ Detail::g_messageScopes.clear();
}
// Reset working state. assertion info will be reset after
@@ -473,8 +483,8 @@ namespace Catch {
m_reporter->benchmarkFailed( error );
}
- void RunContext::pushScopedMessage(MessageInfo const & message) {
- m_messages.push_back(message);
+ void RunContext::pushScopedMessage( MessageInfo const& message ) {
+ Detail::g_messages.push_back( message );
}
void RunContext::popScopedMessage( MessageInfo const& message ) {
@@ -483,16 +493,16 @@ namespace Catch {
// messages than low single digits, so the optimization is tiny,
// and we would have to hand-write the loop to avoid terrible
// codegen of reverse iterators in debug mode.
- m_messages.erase(
- std::find_if( m_messages.begin(),
- m_messages.end(),
+ Detail::g_messages.erase(
+ std::find_if( Detail::g_messages.begin(),
+ Detail::g_messages.end(),
[id = message.sequence]( MessageInfo const& msg ) {
return msg.sequence == id;
} ) );
}
void RunContext::emplaceUnscopedMessage( MessageBuilder&& builder ) {
- m_messageScopes.emplace_back( CATCH_MOVE(builder) );
+ Detail::g_messageScopes.emplace_back( CATCH_MOVE(builder) );
}
std::string RunContext::getCurrentTestName() const {
@@ -651,10 +661,10 @@ namespace Catch {
m_testCaseTracker->close();
handleUnfinishedSections();
- m_messageScopes.clear();
+ Detail::g_messageScopes.clear();
// TBD: At this point, m_messages should be empty. Do we want to
// assert that this is true, or keep the defensive clear call?
- m_messages.clear();
+ Detail::g_messages.clear();
SectionStats testCaseSectionStats(CATCH_MOVE(testCaseSection), assertions, duration, missingAssertions);
m_reporter->sectionEnded(testCaseSectionStats);
diff --git a/src/catch2/internal/catch_run_context.hpp b/src/catch2/internal/catch_run_context.hpp
index 33c0d5a1..39aca522 100644
--- a/src/catch2/internal/catch_run_context.hpp
+++ b/src/catch2/internal/catch_run_context.hpp
@@ -149,9 +149,6 @@ namespace Catch {
Totals m_totals;
Detail::AtomicCounts m_atomicAssertionCount;
IEventListenerPtr m_reporter;
- std::vector m_messages;
- // Owners for the UNSCOPED_X information macro
- std::vector m_messageScopes;
std::vector m_unfinishedSections;
std::vector m_activeSections;
TrackerContext m_trackerContext;