Fix GC background thread start in OOM (#5840)

This change fixes a problem that happened when the `gc_heap::create_bgc_thread`
calls SetupUnstartedThread and it fails to allocate a Thread object and throws.
The GC doesn't expect that and so when the stack is unwound, the `gc_heap::gc_started`
stays set. Now we try to allocate memory for the Throwable for the exception, so we go
to GC heap and since there is not enough space, we end up calling
`gc_heap::try_allocate_more_space`, which calls `gc_heap::wait_for_gc_done`.
And that’s the end of it, since the `gc_started` is still set and we wait forever.

The fix is to catch exception from the SetupUnstartedThread. I have also fixed couple of
places where this method was being called and the exception was not expected.

As an additional cleanup, I have moved the thread creation in GC to the GCToEEInterface.

6 files changed, 49 insertions, 28 deletions

diff --git a/src/vm/assembly.cpp b/src/vm/assembly.cpp
index 3de92206d4..06effd7240 100644
--- a/src/vm/assembly.cpp
+++ b/src/vm/assembly.cpp
@@ -2343,8 +2343,6 @@ static void Stress_Thread_Start (LPVOID lpParameter)
     for (n = 0; n < dwThreads-1; n ++)
     {
         threads[n] = SetupUnstartedThread();
-        if (threads[n] == NULL)
-            COMPlusThrowOM();
 
         threads[n]->m_stressThreadCount = dwThreads/2;
         Stress_Thread_Param *param = ((Stress_Thread_Param*)lpParameter)->Clone();
diff --git a/src/vm/ceemain.cpp b/src/vm/ceemain.cpp
index 9a54adf7b7..f3e8c3b28d 100644
--- a/src/vm/ceemain.cpp
+++ b/src/vm/ceemain.cpp
@@ -3727,8 +3727,7 @@ void InitializeGarbageCollector()
     IfFailThrow(hr);
 
     // Thread for running finalizers...
-    if (FinalizerThread::FinalizerThreadCreate() != 1)
-        ThrowOutOfMemory();
+    FinalizerThread::FinalizerThreadCreate();
 
     // Now we really have fully initialized the garbage collector
     SetGarbageCollectorFullyInitialized();
diff --git a/src/vm/finalizerthread.cpp b/src/vm/finalizerthread.cpp
index 3960719c65..5d51d33cfb 100644
--- a/src/vm/finalizerthread.cpp
+++ b/src/vm/finalizerthread.cpp
@@ -994,13 +994,13 @@ DWORD __stdcall FinalizerThread::FinalizerThreadStart(void *args)
     return 0;
 }
 
-DWORD FinalizerThread::FinalizerThreadCreate()
+void FinalizerThread::FinalizerThreadCreate()
 {
-    DWORD   dwRet = 0;
-
-    // TODO: The following line should be removed after contract violation is fixed.
-    // See bug 27409
-    SCAN_IGNORE_THROW;
+    CONTRACTL{
+        THROWS;
+        GC_TRIGGERS;
+        MODE_ANY;
+    } CONTRACTL_END;
 
 #ifndef FEATURE_PAL
     if (!CLRMemoryHosted())
@@ -1021,9 +1021,6 @@ DWORD FinalizerThread::FinalizerThreadCreate()
 
     _ASSERTE(g_pFinalizerThread == 0);
     g_pFinalizerThread = SetupUnstartedThread();
-    if (g_pFinalizerThread == 0) {
-        return 0;
-    }
 
     // We don't want the thread block disappearing under us -- even if the
     // actual thread terminates.
@@ -1031,7 +1028,7 @@ DWORD FinalizerThread::FinalizerThreadCreate()
 
     if (GetFinalizerThread()->CreateNewThread(0, &FinalizerThreadStart, NULL))
     {
-        dwRet = GetFinalizerThread()->StartThread();
+        DWORD dwRet = GetFinalizerThread()->StartThread();
 
         // When running under a user mode native debugger there is a race
         // between the moment we've created the thread (in CreateNewThread) and 
@@ -1046,10 +1043,6 @@ DWORD FinalizerThread::FinalizerThreadCreate()
         // debugger may have been detached between the time it got the notification
         // and the moment we execute the test below.
         _ASSERTE(dwRet == 1 || dwRet == 2);
-        if (dwRet == 2)
-        {
-            dwRet = 1;
-        }
         
         // workaround wwl: make sure finalizer is ready.  This avoids OOM problem on finalizer
         // thread startup.
@@ -1057,12 +1050,10 @@ DWORD FinalizerThread::FinalizerThreadCreate()
             FinalizerThreadWait(INFINITE);
             if (!s_FinalizerThreadOK)
             {
-                dwRet = 0;
+                ThrowOutOfMemory();
             }
         }
     }
-
-    return dwRet;
 }
 
 void FinalizerThread::SignalFinalizationDone(BOOL fFinalizer)
diff --git a/src/vm/finalizerthread.h b/src/vm/finalizerthread.h
index 42a9e9f250..90dd3e9ec9 100644
--- a/src/vm/finalizerthread.h
+++ b/src/vm/finalizerthread.h
@@ -91,7 +91,7 @@ public:
     static void FinalizeObjectsOnShutdown(LPVOID args);
     static DWORD __stdcall FinalizerThreadStart(void *args);
 
-    static DWORD FinalizerThreadCreate();
+    static void FinalizerThreadCreate();
     static BOOL FinalizerThreadWatchDog();
 };
 
diff --git a/src/vm/gcenv.ee.cpp b/src/vm/gcenv.ee.cpp
index 7cce1716ae..1bb4ee815b 100644
--- a/src/vm/gcenv.ee.cpp
+++ b/src/vm/gcenv.ee.cpp
@@ -746,3 +746,39 @@ void GCToEEInterface::DisablePreemptiveGC(Thread * pThread)
     WRAPPER_NO_CONTRACT;
     pThread->DisablePreemptiveGC();
 }
+
+bool GCToEEInterface::CreateBackgroundThread(Thread** thread, GCBackgroundThreadFunction threadStart, void* arg)
+{
+    CONTRACTL
+    {
+        NOTHROW;
+        GC_TRIGGERS;
+    }
+    CONTRACTL_END;
+
+    Thread* newThread = NULL;
+    EX_TRY
+    {
+        newThread = SetupUnstartedThread(FALSE);
+        *thread = newThread;
+    }
+    EX_CATCH
+    {
+    }
+    EX_END_CATCH(SwallowAllExceptions);
+
+    if ((newThread != NULL) && newThread->CreateNewThread(0, (LPTHREAD_START_ROUTINE)threadStart, arg))
+    {
+        newThread->SetBackground (TRUE, FALSE);
+
+        // 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();
+
+        return true;
+    }
+
+    *thread = NULL;
+    return false;
+}
diff --git a/src/vm/threads.cpp b/src/vm/threads.cpp
index 9e1a528370..5624bb1026 100644
--- a/src/vm/threads.cpp
+++ b/src/vm/threads.cpp
@@ -1118,13 +1118,10 @@ Thread* SetupUnstartedThread(BOOL bRequiresTSL)
     _ASSERTE(ThreadInited());
     Thread* pThread = new Thread();
 
-    if (pThread)
-    {
-        FastInterlockOr((ULONG *) &pThread->m_State,
-                        (Thread::TS_Unstarted | Thread::TS_WeOwn));
+    FastInterlockOr((ULONG *) &pThread->m_State,
+                    (Thread::TS_Unstarted | Thread::TS_WeOwn));
 
-        ThreadStore::AddThread(pThread, bRequiresTSL);
-    }
+    ThreadStore::AddThread(pThread, bRequiresTSL);
 
     return pThread;
 }