Refactor GC background thread creation (#6384)

This change modifies the GCToEEInterface::CreateBackgroundThread so that it returns
a fully initialized and running thread.

1 files changed, 78 insertions, 19 deletions

diff --git a/src/vm/gcenv.ee.cpp b/src/vm/gcenv.ee.cpp
index 5ecae4f8fc..da36e726ba 100644
--- a/src/vm/gcenv.ee.cpp
+++ b/src/vm/gcenv.ee.cpp
@@ -695,12 +695,6 @@ void GCToEEInterface::SyncBlockCachePromotionsGranted(int max_gen)
     SyncBlockCache::GetSyncBlockCache()->GCDone(FALSE, max_gen);
 }
 
-void GCToEEInterface::SetGCSpecial(Thread * pThread)
-{
-    WRAPPER_NO_CONTRACT;
-    pThread->SetGCSpecial(true);
-}
-
 alloc_context * GCToEEInterface::GetAllocContext(Thread * pThread)
 {
     WRAPPER_NO_CONTRACT;
@@ -747,7 +741,47 @@ void GCToEEInterface::DisablePreemptiveGC(Thread * pThread)
     pThread->DisablePreemptiveGC();
 }
 
-bool GCToEEInterface::CreateBackgroundThread(Thread** thread, GCBackgroundThreadFunction threadStart, void* arg)
+struct BackgroundThreadStubArgs
+{
+    Thread* thread;
+    GCBackgroundThreadFunction threadStart;
+    void* arg;
+    CLREvent threadStartedEvent;
+    bool hasStarted;
+};
+
+DWORD BackgroundThreadStub(void* arg)
+{
+    BackgroundThreadStubArgs* stubArgs = (BackgroundThreadStubArgs*)arg;
+    assert (stubArgs->thread != NULL);
+
+    ClrFlsSetThreadType (ThreadType_GC);
+    stubArgs->thread->SetGCSpecial(true);
+    STRESS_LOG_RESERVE_MEM (GC_STRESSLOG_MULTIPLY);
+
+    stubArgs->hasStarted = !!stubArgs->thread->HasStarted(FALSE);
+
+    Thread* thread = stubArgs->thread;
+    GCBackgroundThreadFunction realThreadStart = stubArgs->threadStart;
+    void* realThreadArg = stubArgs->arg;
+    bool hasStarted = stubArgs->hasStarted;
+
+    stubArgs->threadStartedEvent.Set();
+    // The stubArgs cannot be used once the event is set, since that releases wait on the
+    // event in the function that created this thread and the stubArgs go out of scope.
+
+    DWORD result = 0;
+
+    if (hasStarted)
+    {
+        result = realThreadStart(realThreadArg);
+        DestroyThread(thread);
+    }
+
+    return result;
+}
+
+Thread* GCToEEInterface::CreateBackgroundThread(GCBackgroundThreadFunction threadStart, void* arg)
 {
     CONTRACTL
     {
@@ -756,29 +790,54 @@ bool GCToEEInterface::CreateBackgroundThread(Thread** thread, GCBackgroundThread
     }
     CONTRACTL_END;
 
-    Thread* newThread = NULL;
+    BackgroundThreadStubArgs threadStubArgs;
+
+    threadStubArgs.arg = arg;
+    threadStubArgs.thread = NULL;
+    threadStubArgs.threadStart = threadStart;
+    threadStubArgs.hasStarted = false;
+
+    if (!threadStubArgs.threadStartedEvent.CreateAutoEventNoThrow(FALSE))
+    {
+        return NULL;
+    }
+
     EX_TRY
     {
-        newThread = SetupUnstartedThread(FALSE);
-        *thread = newThread;
+        threadStubArgs.thread = SetupUnstartedThread(FALSE);
     }
     EX_CATCH
     {
     }
     EX_END_CATCH(SwallowAllExceptions);
 
-    if ((newThread != NULL) && newThread->CreateNewThread(0, (LPTHREAD_START_ROUTINE)threadStart, arg))
+    if (threadStubArgs.thread == NULL)
     {
-        newThread->SetBackground (TRUE, FALSE);
+        threadStubArgs.threadStartedEvent.CloseEvent();
+        return NULL;
+    }
+
+    if (threadStubArgs.thread->CreateNewThread(0, (LPTHREAD_START_ROUTINE)BackgroundThreadStub, &threadStubArgs))
+    {
+        threadStubArgs.thread->SetBackground (TRUE, FALSE);
+        threadStubArgs.thread->StartThread();
 
-        // wait for the thread to be in its main loop, this is to detect the situation
-        // where someone triggers a GC during dll loading where the loader lock is
-        // held.
-        newThread->StartThread();
+        // Wait for the thread to be in its main loop
+        uint32_t res = threadStubArgs.threadStartedEvent.Wait(INFINITE, FALSE);
+        threadStubArgs.threadStartedEvent.CloseEvent();
+        _ASSERTE(res == WAIT_OBJECT_0);
 
-        return true;
+        if (!threadStubArgs.hasStarted)
+        {
+            // The thread has failed to start and the Thread object was destroyed in the Thread::HasStarted
+            // failure code path.
+            return NULL;
+        }
+
+        return threadStubArgs.thread;
     }
 
-    *thread = NULL;
-    return false;
+    // Destroy the Thread object
+    threadStubArgs.thread->DecExternalCount(FALSE);
+    return NULL;
 }