Refactor GC background thread creation (#6384)

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

Author: janvorli

src/gc/env/gcenv.base.h

@@ -463,13 +463,6 @@ class FinalizerThread
     static HANDLE GetFinalizerEvent();
 };
 
-#ifdef FEATURE_REDHAWK
-typedef uint32_t (__stdcall *BackgroundCallback)(void* pCallbackContext);
-REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalStartBackgroundGCThread(BackgroundCallback callback, void* pCallbackContext);
-#endif // FEATURE_REDHAWK
-
-void DestroyThread(Thread * pThread);
-
 bool IsGCSpecialThread();
 
 inline bool dbgOnly_IsSpecialEEThread()

src/gc/env/gcenv.ee.h

@@ -74,13 +74,12 @@ class GCToEEInterface
     static void EnablePreemptiveGC(Thread * pThread);
     static void DisablePreemptiveGC(Thread * pThread);
 
-    static void SetGCSpecial(Thread * pThread);
     static alloc_context * GetAllocContext(Thread * pThread);
     static bool CatchAtSafePoint(Thread * pThread);
 
     static void GcEnumAllocContexts(enum_alloc_context_func* fn, void* param);
 
-    static bool CreateBackgroundThread(Thread** thread, GCBackgroundThreadFunction threadStart, void* arg);
+    static Thread* CreateBackgroundThread(GCBackgroundThreadFunction threadStart, void* arg);
 };
 
 #endif // __GCENV_EE_H__

src/gc/gc.cpp

@@ -2530,8 +2530,6 @@ gc_history_per_heap gc_heap::bgc_data_per_heap;
 
 BOOL    gc_heap::bgc_thread_running;
 
-CLREvent gc_heap::background_gc_create_event;
-
 CLRCriticalSection gc_heap::bgc_threads_timeout_cs;
 
 CLREvent gc_heap::gc_lh_block_event;
@@ -7136,7 +7134,7 @@ int gc_heap::grow_brick_card_tables (uint8_t* start,
         dprintf (GC_TABLE_LOG, ("Table alloc for %Id bytes: [%Ix, %Ix[",
                                  alloc_size, (size_t)mem, (size_t)((uint8_t*)mem+alloc_size)));
 
-        {
+        {   
             // mark array will be committed separately (per segment).
             size_t commit_size = alloc_size - ms;
 
@@ -24851,27 +24849,6 @@ void __stdcall gc_heap::gc_thread_stub (void* arg)
 uint32_t __stdcall gc_heap::bgc_thread_stub (void* arg)
 {
     gc_heap* heap = (gc_heap*)arg;
-
-    // TODO: need to check if we are still fine for these APIs:
-    // Thread::BeginThreadAffinity
-    // Thread::EndThreadAffinity()
-    // since now GC threads can be managed threads.
-    ClrFlsSetThreadType (ThreadType_GC);
-    assert (heap->bgc_thread != NULL);
-    GCToEEInterface::SetGCSpecial(heap->bgc_thread);
-    STRESS_LOG_RESERVE_MEM (GC_STRESSLOG_MULTIPLY);
-
-    // We commit the thread's entire stack to ensure we're robust in low memory conditions.
-    /*
-    BOOL fSuccess = Thread::CommitThreadStack();
-
-    if (!fSuccess)
-    {
-        // For background GC we revert to doing a blocking GC.
-        return 0;
-    }
-    */
-
     return heap->bgc_thread_function();
 }
 #ifdef _MSC_VER
@@ -26652,39 +26629,10 @@ BOOL gc_heap::create_bgc_thread(gc_heap* gh)
 
     //dprintf (2, ("Creating BGC thread"));
 
-    if (GCToEEInterface::CreateBackgroundThread(&gh->bgc_thread, gh->bgc_thread_stub, gh))
-    {
-        dprintf (2, ("waiting for the thread to reach its main loop"));
-        // In chk builds this can easily time out since
-        // now we need to set the thread up into a managed thead. 
-        // And since it's a managed thread we also need to make sure that we don't
-        // clean up here and are still executing code on that thread (it'll
-        // trigger all sorts of asserts.
-        //uint32_t res = gh->background_gc_create_event.Wait(20,FALSE);
-        uint32_t res = gh->background_gc_create_event.Wait(INFINITE,FALSE);
-        if (res == WAIT_TIMEOUT)
-        {
-            dprintf (2, ("waiting for the thread to reach its main loop Timeout."));
-            goto cleanup;
-        }
-        if (!gh->bgc_thread_running)
-        {
-            dprintf(2, ("background GC thread failed to start."));
-            goto cleanup;
-        }
-        //dprintf (2, ("waiting for the thread to reach its main loop Done."));
+    gh->bgc_thread = GCToEEInterface::CreateBackgroundThread(gh->bgc_thread_stub, gh);
+    gh->bgc_thread_running = (gh->bgc_thread != NULL);    
 
-        return TRUE;
-    }
-
-cleanup:
-
-    if (gh->bgc_thread)
-    {
-        gh->bgc_thread = 0;
-    }
-
-    return FALSE;
+    return gh->bgc_thread_running;
 }
 
 BOOL gc_heap::create_bgc_threads_support (int number_of_heaps)
@@ -26751,11 +26699,6 @@ BOOL gc_heap::create_bgc_thread_support()
         goto cleanup;
     }
 
-    if (!background_gc_create_event.CreateAutoEventNoThrow(FALSE))
-    {
-        goto cleanup;
-    }
-
     //needs to have room for enough smallest objects fitting on a page
     parr = new (nothrow) (uint8_t* [1 + page_size / MIN_OBJECT_SIZE]);
     if (!parr)
@@ -26775,10 +26718,6 @@ BOOL gc_heap::create_bgc_thread_support()
         {
             gc_lh_block_event.CloseEvent();
         }
-        if (background_gc_create_event.IsValid())
-        {
-            background_gc_create_event.CloseEvent();
-        }
     }
 
     return ret;
@@ -26860,7 +26799,6 @@ void gc_heap::kill_gc_thread()
     // In the first stage, we do minimum work, and call ExitProcess at the end.
     // In the secodn stage, we have the Loader lock and only one thread is
     // alive.  Hence we do not need to kill gc thread.
-    DestroyThread (bgc_thread);
     background_gc_done_event.CloseEvent();
     gc_lh_block_event.CloseEvent();
     bgc_start_event.CloseEvent();
@@ -26877,28 +26815,11 @@ uint32_t gc_heap::bgc_thread_function()
     dprintf (3, ("gc_thread thread starting..."));
 
     BOOL do_exit = FALSE;
-    Thread* thread_to_destroy = 0;
 
-#ifndef FEATURE_REDHAWK
-    // see comments in create_bgc_thread - we need
-    // to make sure that thread doesn't clean up this thread
-    // while we run code here. 
-    if (!bgc_thread->HasStarted(FALSE))
-    {
-        dprintf (2, ("HasStarted failed"));
-        bgc_thread_running = FALSE;
-        background_gc_create_event.Set();
-        return 0;
-    }
-#endif //FEATURE_REDHAWK
-
-    bgc_thread_running = TRUE;
     Thread* current_thread = GetThread();
     BOOL cooperative_mode = TRUE;
     bgc_thread_id.SetToCurrentThread();
     dprintf (1, ("bgc_thread_id is set to %x", (uint32_t)GCToOSInterface::GetCurrentThreadIdForLogging()));
-    //this also indicates that the thread is ready.
-    background_gc_create_event.Set();
     while (1)
     {
         // Wait for work to do...
@@ -26938,10 +26859,6 @@ uint32_t gc_heap::bgc_thread_function()
             {
                 dprintf (2, ("GC thread exiting"));
                 bgc_thread_running = FALSE;
-                // We can't call DestroyThread here 'cause EnterCriticalSection 
-                // increases the thread's m_dwLockCount and DestroyThread will
-                // assert if the lock count is not 0.
-                thread_to_destroy = bgc_thread;
                 bgc_thread = 0;
                 bgc_thread_id.Clear();
                 do_exit = TRUE;
@@ -27045,11 +26962,6 @@ uint32_t gc_heap::bgc_thread_function()
         //gc_heap::disable_preemptive (current_thread, TRUE);
     }
 
-    if (thread_to_destroy)
-    {
-        DestroyThread(thread_to_destroy);
-    }
-    
     FireEtwGCTerminateConcurrentThread_V1(GetClrInstanceId());
 
     dprintf (3, ("bgc_thread thread exiting"));

src/gc/gcpriv.h

@@ -3104,9 +3104,6 @@ class gc_heap
     PER_HEAP_ISOLATED
     CLREvent background_gc_done_event;
 
-    PER_HEAP
-    CLREvent background_gc_create_event;
-
     PER_HEAP_ISOLATED
     CLREvent ee_proceed_event;
 

src/gc/sample/gcenv.ee.cpp

@@ -184,11 +184,6 @@ void GCToEEInterface::DisablePreemptiveGC(Thread * pThread)
     pThread->DisablePreemptiveGC();
 }
 
-void GCToEEInterface::SetGCSpecial(Thread * pThread)
-{
-    pThread->SetGCSpecial(true);
-}
-
 alloc_context * GCToEEInterface::GetAllocContext(Thread * pThread)
 {
     return pThread->GetAllocContext();
@@ -220,10 +215,10 @@ void GCToEEInterface::SyncBlockCachePromotionsGranted(int /*max_gen*/)
 {
 }
 
-bool GCToEEInterface::CreateBackgroundThread(Thread** thread, GCBackgroundThreadFunction threadStart, void* arg)
+Thread* GCToEEInterface::CreateBackgroundThread(GCBackgroundThreadFunction threadStart, void* arg)
 {
     // TODO: Implement for background GC
-    return false;
+    return NULL;
 }
 
 void FinalizerThread::EnableFinalization()

src/gc/sample/gcenv.unix.cpp

@@ -164,11 +164,6 @@ uint32_t CLREventStatic::Wait(uint32_t dwMilliseconds, bool bAlertable)
 }
 #endif // 0
 
-void DestroyThread(Thread * pThread)
-{
-    // TODO: implement
-}
-
 bool __SwitchToThread(uint32_t dwSleepMSec, uint32_t dwSwitchCount)
 {
     return sched_yield() == 0;
@@ -245,10 +240,6 @@ void ThreadStore::AttachCurrentThread(bool fAcquireThreadStoreLock)
     g_pThreadList = pThread;
 }
 #endif // 0
-void DestroyThread(Thread * pThread)
-{
-    // TODO: Implement
-}
 
 #if 0 
 void GCToEEInterface::SuspendEE(GCToEEInterface::SUSPEND_REASON reason)
@@ -292,12 +283,6 @@ void FinalizerThread::EnableFinalization()
     // TODO: Implement for finalization
 }
 
-bool PalStartBackgroundGCThread(BackgroundCallback callback, void* pCallbackContext)
-{
-    // TODO: Implement for background GC
-    return false;
-}
-
 bool IsGCSpecialThread()
 {
     // TODO: Implement for background GC

src/gc/sample/gcenv.windows.cpp

@@ -455,8 +455,3 @@ void CLRCriticalSection::Leave()
 {
     ::LeaveCriticalSection(&m_cs);
 }
-
-void DestroyThread(Thread * pThread)
-{
-    // TODO: implement
-}