diff options
author | Jan Vorlicek <janvorli@microsoft.com> | 2016-07-21 18:42:40 +0200 |
---|---|---|
committer | Jan Kotas <jkotas@microsoft.com> | 2016-07-21 09:42:40 -0700 |
commit | 3b5550f9892bce0e17cc50f26e655f2215ce3c9e (patch) | |
tree | a275d330257fd6f705b227ccdf7d64ebdf90e26a /src | |
parent | bfe11c1f9326134daf9dd6f471b1d0b947437cf0 (diff) | |
download | coreclr-3b5550f9892bce0e17cc50f26e655f2215ce3c9e.tar.gz coreclr-3b5550f9892bce0e17cc50f26e655f2215ce3c9e.tar.bz2 coreclr-3b5550f9892bce0e17cc50f26e655f2215ce3c9e.zip |
Refactor GC background thread creation (#6384)
This change modifies the GCToEEInterface::CreateBackgroundThread so that it returns
a fully initialized and running thread.
Diffstat (limited to 'src')
-rw-r--r-- | src/gc/env/gcenv.base.h | 7 | ||||
-rw-r--r-- | src/gc/env/gcenv.ee.h | 3 | ||||
-rw-r--r-- | src/gc/gc.cpp | 96 | ||||
-rw-r--r-- | src/gc/gcpriv.h | 3 | ||||
-rw-r--r-- | src/gc/sample/gcenv.ee.cpp | 9 | ||||
-rw-r--r-- | src/gc/sample/gcenv.unix.cpp | 15 | ||||
-rw-r--r-- | src/gc/sample/gcenv.windows.cpp | 5 | ||||
-rw-r--r-- | src/vm/gcenv.ee.cpp | 97 |
8 files changed, 85 insertions, 150 deletions
diff --git a/src/gc/env/gcenv.base.h b/src/gc/env/gcenv.base.h index 329ac6e1dc..83726f4eab 100644 --- a/src/gc/env/gcenv.base.h +++ b/src/gc/env/gcenv.base.h @@ -463,13 +463,6 @@ public: 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() diff --git a/src/gc/env/gcenv.ee.h b/src/gc/env/gcenv.ee.h index 42f47c4b18..0c1fd4988a 100644 --- a/src/gc/env/gcenv.ee.h +++ b/src/gc/env/gcenv.ee.h @@ -74,13 +74,12 @@ public: 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__ diff --git a/src/gc/gc.cpp b/src/gc/gc.cpp index a3792e1516..af2d1e7915 100644 --- a/src/gc/gc.cpp +++ b/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 @@ cleanup: { 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")); diff --git a/src/gc/gcpriv.h b/src/gc/gcpriv.h index 42ca64ce76..03a23454a0 100644 --- a/src/gc/gcpriv.h +++ b/src/gc/gcpriv.h @@ -3104,9 +3104,6 @@ protected: PER_HEAP_ISOLATED CLREvent background_gc_done_event; - PER_HEAP - CLREvent background_gc_create_event; - PER_HEAP_ISOLATED CLREvent ee_proceed_event; diff --git a/src/gc/sample/gcenv.ee.cpp b/src/gc/sample/gcenv.ee.cpp index cdc0a34972..330564a380 100644 --- a/src/gc/sample/gcenv.ee.cpp +++ b/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() diff --git a/src/gc/sample/gcenv.unix.cpp b/src/gc/sample/gcenv.unix.cpp index 53c821a5bb..c8663fba64 100644 --- a/src/gc/sample/gcenv.unix.cpp +++ b/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 diff --git a/src/gc/sample/gcenv.windows.cpp b/src/gc/sample/gcenv.windows.cpp index 1324bd3869..e35af6b6a0 100644 --- a/src/gc/sample/gcenv.windows.cpp +++ b/src/gc/sample/gcenv.windows.cpp @@ -455,8 +455,3 @@ void CLRCriticalSection::Leave() { ::LeaveCriticalSection(&m_cs); } - -void DestroyThread(Thread * pThread) -{ - // TODO: implement -} 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; } |