work around doctest shutdown issues with static CRT (#784)

* tweaked doctest.h to avoid shutdown issues due to thread_local variables running destructors after the main thread has torn down everything including the heap
* disabled zenserver exit thread waiting since doctest should hopefully not be causing issues during shutdown anymore after my workaround

This should help reduce the duration of tests spawning lots of server instances

2 files changed, 32 insertions, 6 deletions

diff --git a/src/zenserver/main.cpp b/src/zenserver/main.cpp
index ee783d2a6..571dd3b4f 100644
--- a/src/zenserver/main.cpp
+++ b/src/zenserver/main.cpp
@@ -267,6 +267,14 @@ main(int argc, char* argv[])
 	using namespace zen;
 	using namespace std::literals;
 
+	// note: doctest has locally (in thirdparty) been fixed to not cause shutdown
+	// 		 crashes due to TLS destructors
+	//
+	// mimalloc on the other hand might still be causing issues, in which case
+	// we should work out either how to eliminate the mimalloc dependency or how
+	// to configure it in a way that doesn't cause shutdown issues
+
+#if 0 	
 	auto _ = zen::MakeGuard([] {
 		// Allow some time for worker threads to unravel, in an effort
 		// to prevent shutdown races in TLS object destruction, mainly due to
@@ -277,6 +285,7 @@ main(int argc, char* argv[])
 		// shutdown crashes observed in some situations.
 		WaitForThreads(1000);
 	});
+#endif
 
 	enum
 	{
diff --git a/thirdparty/doctest/doctest/doctest.h b/thirdparty/doctest/doctest/doctest.h
index f297fa0b2..5299d9c1a 100644
--- a/thirdparty/doctest/doctest/doctest.h
+++ b/thirdparty/doctest/doctest/doctest.h
@@ -3337,7 +3337,8 @@ namespace {
 } // namespace

 

 namespace detail {

-    DOCTEST_THREAD_LOCAL class

+    // Leaked heap allocation - see g_infoContexts comment above.

+    class ThreadLocalStringStream

     {

         std::vector<std::streampos> stack;

         std::stringstream           ss;

@@ -3358,7 +3359,12 @@ namespace detail {
             ss.rdbuf()->pubseekpos(pos, std::ios::in | std::ios::out);

             return String(ss, sz);

         }

-    } g_oss;

+    };

+    static ThreadLocalStringStream& g_oss_ref() {

+        DOCTEST_THREAD_LOCAL auto* p = new ThreadLocalStringStream();

+        return *p;

+    }

+    #define g_oss g_oss_ref() // NOLINT

 

     std::ostream* tlssPush() {

         return g_oss.push();

@@ -4600,7 +4606,14 @@ namespace detail {
             getExceptionTranslators().push_back(et);

     }

 

-    DOCTEST_THREAD_LOCAL std::vector<IContextScope*> g_infoContexts; // for logging with INFO()

+    // Use a leaked heap allocation for thread-local state to avoid calling destructors

+    // during shutdown. With static CRT linking, thread-local destructors can run after

+    // the CRT has already been torn down on secondary threads, causing crashes.

+    static std::vector<IContextScope*>& g_infoContexts_ref() {

+        DOCTEST_THREAD_LOCAL auto* p = new std::vector<IContextScope*>();

+        return *p;

+    }

+    #define g_infoContexts g_infoContexts_ref() // NOLINT

 

     ContextScopeBase::ContextScopeBase() {

         g_infoContexts.push_back(this);

@@ -6440,13 +6453,18 @@ namespace {
 #ifdef DOCTEST_PLATFORM_WINDOWS

     struct DebugOutputWindowReporter : public ConsoleReporter

     {

-        DOCTEST_THREAD_LOCAL static std::ostringstream oss;

+        // Leaked heap allocation - see g_infoContexts comment in detail namespace.

+        static std::ostringstream& oss_ref() {

+            DOCTEST_THREAD_LOCAL auto* p = new std::ostringstream();

+            return *p;

+        }

 

         DebugOutputWindowReporter(const ContextOptions& co)

-                : ConsoleReporter(co, oss) {}

+                : ConsoleReporter(co, oss_ref()) {}

 

 #define DOCTEST_DEBUG_OUTPUT_REPORTER_OVERRIDE(func, type, arg)                                    \

     void func(type arg) override {                                                                 \

+        auto& oss = oss_ref();                                                                     \

         bool with_col = g_no_colors;                                                               \

         g_no_colors   = false;                                                                     \

         ConsoleReporter::func(arg);                                                                \

@@ -6470,7 +6488,6 @@ namespace {
         DOCTEST_DEBUG_OUTPUT_REPORTER_OVERRIDE(test_case_skipped, const TestCaseData&, in)

     };

 

-    DOCTEST_THREAD_LOCAL std::ostringstream DebugOutputWindowReporter::oss;

 #endif // DOCTEST_PLATFORM_WINDOWS

 

     // the implementation of parseOption()