summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorJan Vorlicek <janvorli@microsoft.com>2016-07-21 18:42:40 +0200
committerJan Kotas <jkotas@microsoft.com>2016-07-21 09:42:40 -0700
commit3b5550f9892bce0e17cc50f26e655f2215ce3c9e (patch)
treea275d330257fd6f705b227ccdf7d64ebdf90e26a /src
parentbfe11c1f9326134daf9dd6f471b1d0b947437cf0 (diff)
downloadcoreclr-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.h7
-rw-r--r--src/gc/env/gcenv.ee.h3
-rw-r--r--src/gc/gc.cpp96
-rw-r--r--src/gc/gcpriv.h3
-rw-r--r--src/gc/sample/gcenv.ee.cpp9
-rw-r--r--src/gc/sample/gcenv.unix.cpp15
-rw-r--r--src/gc/sample/gcenv.windows.cpp5
-rw-r--r--src/vm/gcenv.ee.cpp97
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;
}