From b3354ee63584f5adb1f9bc057255c25ebf469dd9 Mon Sep 17 00:00:00 2001 From: Brian Fiete Date: Mon, 31 Aug 2020 11:29:11 -0700 Subject: [PATCH] Fixed GC race condition starting an autodelete thread --- BeefLibs/corlib/src/GC.bf | 3 +++ BeefLibs/corlib/src/Threading/Thread.bf | 2 ++ BeefRT/dbg/gc.cpp | 26 ++++++++++++++++++++++++- BeefRT/dbg/gc.h | 7 +++++-- BeefRT/rt/Thread.cpp | 8 ++++---- 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/BeefLibs/corlib/src/GC.bf b/BeefLibs/corlib/src/GC.bf index 1e0568d9..35660688 100644 --- a/BeefLibs/corlib/src/GC.bf +++ b/BeefLibs/corlib/src/GC.bf @@ -131,6 +131,8 @@ namespace System public extern static void SetCollectFreeThreshold(int freeBytes); // -1 to disable, 0 to trigger collection after every single free. Defaults to 64MB [CallingConvention(.Cdecl)] public extern static void SetMaxPausePercentage(int maxPausePercentage); // 0 = disabled. Defaults to 20. + [CallingConvention(.Cdecl)] + extern static void AddPendingThread(void* internalThread); #else public static void Collect(bool async = true) {} private static void MarkAllStaticMembers() {} @@ -141,6 +143,7 @@ namespace System public static void SetAutoCollectPeriod(int periodMS) {} public static void SetCollectFreeThreshold(int freeBytes) {} public static void SetMaxPausePercentage(int maxPausePercentage) {} + extern static void AddPendingThread(void* internalThreadInfo) {} #endif static void MarkDerefedObject(Object* obj) diff --git a/BeefLibs/corlib/src/Threading/Thread.bf b/BeefLibs/corlib/src/Threading/Thread.bf index 5ef54934..a357cb84 100644 --- a/BeefLibs/corlib/src/Threading/Thread.bf +++ b/BeefLibs/corlib/src/Threading/Thread.bf @@ -41,6 +41,8 @@ namespace System.Threading static void Thread_SetInternalThread(Object thread, void* internalThread) { + if (internalThread != null) + GC.[Friend]AddPendingThread(internalThread); ((Thread)thread).[Friend]mInternalThread = (int)internalThread; } diff --git a/BeefRT/dbg/gc.cpp b/BeefRT/dbg/gc.cpp index 5d6f0263..dbe01611 100644 --- a/BeefRT/dbg/gc.cpp +++ b/BeefRT/dbg/gc.cpp @@ -1389,7 +1389,17 @@ bool BFGC::ScanThreads() while (true) { ThreadInfo* thread = NULL; + + /// + { + AutoCrit autoCrit(mCritSect); + for (auto& kv : mPendingThreads) + { + MarkFromGCThread(kv.mValue->mThread); + } + } + /// { AutoCrit autoCrit(mCritSect); if (threadIdx >= mThreadList.size()) @@ -1927,7 +1937,7 @@ void BFGC::ThreadStopped(BfDbgInternalThread* thread) void BFGC::ThreadStarted() { Beefy::AutoCrit autoCrit(mCritSect); - + ThreadInfo* thread = new ThreadInfo(); thread->mRunning = true; thread->mThreadHandle = BfpThread_GetCurrent(); @@ -1938,6 +1948,7 @@ void BFGC::ThreadStarted() thread->CalcStackStart(); mThreadList.Add(thread); + mPendingThreads.Remove(thread->mThreadId); ThreadInfo::sCurThreadInfo = thread; } @@ -2028,6 +2039,14 @@ void BFGC::RemoveStackMarkableObject(bf::System::Object* obj) threadInfo->mStackMarkableObjects.RemoveAtFast(stackIdx); } +void BFGC::AddPendingThread(BfInternalThread* internalThread) +{ + if (internalThread->mThread == 0) + return; + Beefy::AutoCrit autoCrit(mCritSect); + mPendingThreads.TryAdd(internalThread->mThreadId, internalThread); +} + void BFGC::Shutdown() { if (mShutdown) @@ -2710,6 +2729,11 @@ void GC::AddStackMarkableObject(Object* obj) gBFGC.AddStackMarkableObject(obj); } +void GC::AddPendingThread(void* internalThreadInfo) +{ + gBFGC.AddPendingThread((BfInternalThread*)internalThreadInfo); +} + void GC::RemoveStackMarkableObject(Object* obj) { gBFGC.RemoveStackMarkableObject(obj); diff --git a/BeefRT/dbg/gc.h b/BeefRT/dbg/gc.h index fe1eae19..15ccc2b5 100644 --- a/BeefRT/dbg/gc.h +++ b/BeefRT/dbg/gc.h @@ -199,7 +199,7 @@ public: bool WantsSuspend(); void CalcStackStart(); }; - + struct RawLeakInfo { bf::System::DbgRawAllocData* mRawAllocData; @@ -324,6 +324,7 @@ public: int mCurPendingGCSize; int mMaxPendingGCSize; Beefy::Array mThreadList; + Beefy::Dictionary mPendingThreads; int mCurMutatorMarkCount; int mCurGCMarkCount; int mCurGCObjectQueuedCount; @@ -390,6 +391,7 @@ public: void StopCollecting(); void AddStackMarkableObject(bf::System::Object* obj); void RemoveStackMarkableObject(bf::System::Object* obj); + void AddPendingThread(BfInternalThread* internalThread); void Shutdown(); void InitDebugDump(); void EndDebugDump(); @@ -463,6 +465,7 @@ namespace bf BFRT_EXPORT static void StopCollecting(); BFRT_EXPORT static void AddStackMarkableObject(Object* obj); BFRT_EXPORT static void RemoveStackMarkableObject(Object* obj); + BFRT_EXPORT static void AddPendingThread(void* internalThreadInfo); public: BFRT_EXPORT static void Shutdown(); @@ -483,7 +486,7 @@ namespace bf BFRT_EXPORT static void SetAutoCollectPeriod(intptr periodMS); BFRT_EXPORT static void SetCollectFreeThreshold(intptr freeBytes); BFRT_EXPORT static void SetMaxPausePercentage(intptr maxPausePercentage); - BFRT_EXPORT static void SetMaxRawDeferredObjectFreePercentage(intptr maxPercentage); + BFRT_EXPORT static void SetMaxRawDeferredObjectFreePercentage(intptr maxPercentage); }; } } diff --git a/BeefRT/rt/Thread.cpp b/BeefRT/rt/Thread.cpp index e0f8d0c1..9eeb9718 100644 --- a/BeefRT/rt/Thread.cpp +++ b/BeefRT/rt/Thread.cpp @@ -191,10 +191,10 @@ void Thread::StartInternal() BfpSystem_InterlockedExchangeAdd32((uint32*)&gLiveThreadCount, 1); BfInternalThread* internalThread = SetupInternalThread(); - SetInternalThread(internalThread); - - internalThread->mThread = this; - internalThread->mThreadHandle = BfpThread_Create(CStartProc, (void*)this, GetMaxStackSize(), BfpThreadCreateFlag_StackSizeReserve, &internalThread->mThreadId); + internalThread->mThread = this; + internalThread->mThreadHandle = BfpThread_Create(CStartProc, (void*)this, GetMaxStackSize(), (BfpThreadCreateFlags)(BfpThreadCreateFlag_StackSizeReserve | BfpThreadCreateFlag_Suspended), &internalThread->mThreadId); + SetInternalThread(internalThread); + BfpThread_Resume(internalThread->mThreadHandle, NULL); } int Thread::GetThreadId()