Merge pull request #483 from GeneralsOnlineDevelopmentTeam/seer/bugfix/thread-list-cs-robustness

bugfix(threading): Ensure thread list critical section remains valid during CRT shutdown

Author: x64-dev

Core/Libraries/Source/WWVegas/WWLib/Except.cpp

@@ -114,20 +114,72 @@ int ExceptionRecursions = -1;
 DynamicVectorClass<ThreadInfoType*> ThreadList;
 
 /*
-** Critical section to protect ThreadList from concurrent access.
-** This prevents race conditions when threads register/unregister while
-** another thread is accessing the list (e.g., during exception handling or shutdown).
+** Returns the CRITICAL_SECTION used to protect ThreadList.
 **
-** Intentionally heap-allocated and never freed: threads may call Unregister_Thread_ID
-** after C++ global destructors have run, so a static-lifetime object would already be
-** destroyed by that point, causing a use-after-free crash.
+** Allocated from the Windows process heap (not the CRT heap) and never freed.
+** The CRT heap can be torn down during application shutdown before all threads
+** have exited. If a thread calls Unregister_Thread_ID after the CRT heap is
+** destroyed, any CRITICAL_SECTION allocated via _aligned_malloc (which uses
+** the CRT heap) would already be invalid memory, causing an access violation
+** when EnterCriticalSection dereferences it.
+**
+** The Windows process heap (GetProcessHeap) outlives the CRT heap and is only
+** reclaimed when the process terminates, so this CRITICAL_SECTION remains valid
+** for the entire process lifetime regardless of CRT shutdown order.
+**
+** Thread-safe one-time initialization is achieved via
+** InterlockedCompareExchangePointer, which works on all supported Windows versions.
 */
-static CriticalSectionClass& GetThreadListLock()
+static CRITICAL_SECTION* GetThreadListCS()
 {
-	static CriticalSectionClass* lock = new CriticalSectionClass();
-	return *lock;
+	static CRITICAL_SECTION* volatile s_cs = nullptr;
+
+	if (s_cs == nullptr) {
+		CRITICAL_SECTION* cs = reinterpret_cast<CRITICAL_SECTION*>(
+			HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(CRITICAL_SECTION)));
+		if (cs != nullptr) {
+			InitializeCriticalSection(cs);
+		}
+		// Race-free handoff: only one thread's allocation wins; the loser is discarded.
+		if (InterlockedCompareExchangePointer(
+				reinterpret_cast<volatile PVOID*>(&s_cs), cs, nullptr) != nullptr) {
+			// Another thread initialized it first; discard our copy.
+			if (cs != nullptr) {
+				DeleteCriticalSection(cs);
+				HeapFree(GetProcessHeap(), 0, cs);
+			}
+		}
+	}
+
+	return s_cs;
 }
 
+/*
+** RAII lock guard for the raw ThreadList CRITICAL_SECTION.
+** Replaces CriticalSectionClass::LockClass for the thread-list lock so that
+** the CriticalSectionClass wrapper (and its _aligned_malloc-based handle) is
+** never used for this particular, shutdown-sensitive critical path.
+*/
+struct ScopedThreadListLock
+{
+	explicit ScopedThreadListLock(CRITICAL_SECTION* cs) : cs_(cs)
+	{
+		if (cs_ != nullptr) {
+			EnterCriticalSection(cs_);
+		}
+	}
+	~ScopedThreadListLock()
+	{
+		if (cs_ != nullptr) {
+			LeaveCriticalSection(cs_);
+		}
+	}
+private:
+	CRITICAL_SECTION* cs_;
+	ScopedThreadListLock(const ScopedThreadListLock&);
+	ScopedThreadListLock& operator=(const ScopedThreadListLock&);
+};
+
 /*
 ** Definitions to allow run-time linking to the Imagehlp.dll functions.
 **
@@ -905,7 +957,7 @@ void Register_Thread_ID(unsigned long thread_id, char *thread_name, bool main_th
 {
 	WWMEMLOG(MEM_GAMEDATA);
 	if (thread_name) {
-		CriticalSectionClass::LockClass lock(GetThreadListLock());
+		ScopedThreadListLock lock(GetThreadListCS());
 
 		/*
 		** See if we already know about this thread. Maybe just the thread_id changed.
@@ -1016,7 +1068,7 @@ HANDLE Get_Thread_Handle(int thread_index)
  *=============================================================================================*/
 void Unregister_Thread_ID(unsigned long thread_id, char *thread_name)
 {
-	CriticalSectionClass::LockClass lock(GetThreadListLock());
+	ScopedThreadListLock lock(GetThreadListCS());
 
 	for (int i=0 ; i<ThreadList.Count() ; i++) {
 		if (strcmp(thread_name, ThreadList[i]->ThreadName) == 0) {
@@ -1046,7 +1098,7 @@ void Unregister_Thread_ID(unsigned long thread_id, char *thread_name)
  *=============================================================================================*/
 unsigned long Get_Main_Thread_ID()
 {
-	CriticalSectionClass::LockClass lock(GetThreadListLock());
+	ScopedThreadListLock lock(GetThreadListCS());
 
 	for (int i=0 ; i<ThreadList.Count() ; i++) {
 		if (ThreadList[i]->Main) {