Fix lifetime issues when using UNSCOPED_X message macros

The original implementation of `UNSCOPED_X` message macros used a
clever hack to make the original implementation simpler: construct
an instance of `ScopedMessage` to manage its lifetime, but store
it in a vector, so its lifetime is not actually scope-based, and
we can manage it through the vector instance.

This hack made it so that the lifetime of the vector that manages
the fake `ScopedMessage`s must be outlived by the vector with the
actual messages. Originally this wasn't a problem, because they both
lived inside the run context instance. However, since then these
vectors became globals and thread-local. When this happened, it
still wasn't a problem; the two globals were declared in the right
order, so they were destroyed in the right order as well.

Then, in f80956a43a3e276686ed59531b41617282d75a0c, these globals
were turned into magic static globals to improve their behaviour
in MSVC's Debug build mode. This caused their lifetimes to be
runtime-dependent; if a specific test thread added its first scoped
message before it added first unscoped message, the lifetimes
would be correct. If it instead added first unscoped message
before adding first scoped message, then there **might** be
invalid reads during thread destruction.

The fix is simple: do things properly and manage the lifetime of
messages in `UNSCOPED_X` explicitly. Then we don't have to deal
with the destruction of fake `ScopedMessage`s while the thread is
being destroyed, and the lifetime of the two vectors is no longer
tied together.

I also threw them both into a new type, to encapsulate some of the
unscoped message logic.

2 files changed, 33 insertions, 3 deletions

diff --git a/tests/ExtraTests/CMakeLists.txt b/tests/ExtraTests/CMakeLists.txt
index 94eab4f0..60993b13 100644
--- a/tests/ExtraTests/CMakeLists.txt
+++ b/tests/ExtraTests/CMakeLists.txt
@@ -572,11 +572,20 @@ add_executable(ThreadSafetyTests
 )
 target_link_libraries(ThreadSafetyTests Catch2_buildall_interface)
 target_compile_definitions(ThreadSafetyTests PUBLIC CATCH_CONFIG_EXPERIMENTAL_THREAD_SAFE_ASSERTIONS)
-add_test(NAME ThreadSafetyTests
+
+add_test(NAME ThreadSafetyTests::ScopedMessagesAndAssertions
   COMMAND ThreadSafetyTests -r compact "Failed REQUIRE in the main thread is fine"
 )
-set_tests_properties(ThreadSafetyTests
+set_tests_properties(ThreadSafetyTests::ScopedMessagesAndAssertions
+  PROPERTIES
+    PASS_REGULAR_EXPRESSION "assertions: 801 \\| 400 passed \\| 401 failed as expected"
+    RUN_SERIAL ON
+)
+add_test(NAME ThreadSafetyTests::UnscopedMessagesAndAssertions
+  COMMAND ThreadSafetyTests -r compact "Using unscoped messages in sibling threads"
+)
+set_tests_properties(ThreadSafetyTests::UnscopedMessagesAndAssertions
   PROPERTIES
-    PASS_REGULAR_EXPRESSION "assertions: 801 | 400 passed | 401 failed"
+    PASS_REGULAR_EXPRESSION "assertions: 401 \\| 401 failed as expected"
     RUN_SERIAL ON
 )
diff --git a/tests/ExtraTests/X94-ThreadSafetyTests.cpp b/tests/ExtraTests/X94-ThreadSafetyTests.cpp
index d391417b..3f8ba9fa 100644
--- a/tests/ExtraTests/X94-ThreadSafetyTests.cpp
+++ b/tests/ExtraTests/X94-ThreadSafetyTests.cpp
@@ -41,3 +41,24 @@ TEST_CASE( "Failed REQUIRE in the main thread is fine", "[!shouldfail]" ) {
 
     REQUIRE( false );
 }
+
+TEST_CASE( "Using unscoped messages in sibling threads", "[!shouldfail]" ) {
+    std::vector<std::thread> threads;
+    for ( size_t t = 0; t < 4; ++t) {
+        threads.emplace_back( [t]() {
+            UNSCOPED_INFO("thread " << t << " start");
+            for (size_t i = 0; i < 100; ++i) {
+                for (size_t j = 0; j < 4; ++j) {
+                    UNSCOPED_INFO("t=" << i << ", " << j);
+                }
+                CHECK( false );
+            }
+        } );
+    }
+
+    for (auto& t : threads) {
+        t.join();
+    }
+
+    REQUIRE( false );
+}