Skip to content

Conversation

@alexey-zakharov
Copy link

@alexey-zakharov alexey-zakharov commented Mar 15, 2024

Motivation:

Fix crash in ThreadLocalBlock::FreeTLM code on thread exit which happens during Assembly unloading when Assembly contains a class with a ThreadStatic variable.

Details:

There seem to be a race condition between LoaderAllocator cleanup during garbage collection and thread locals cleanup on thread exit. Racing stacks are the following:

ALC/LoaderAllocator cleanup

 	coreclr.dll!EEToProfInterfaceImpl::ModuleUnloadStarted(unsigned __int64 moduleId) Line 3735	C++	Symbols loaded.
 	coreclr.dll!ProfControlBlock::DoProfilerCallbackHelper<int (__cdecl*)(ProfilerInfo *),long (__cdecl*)(EEToProfInterfaceImpl *,unsigned __int64),unsigned __int64>(ProfilerInfo * pProfilerInfo, int(*)(ProfilerInfo *) condition, HRESULT(*)(EEToProfInterfaceImpl *, unsigned __int64) callback, HRESULT * pHR, unsigned __int64 <args_0>) Line 284	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!ProfControlBlock::DoOneProfilerIteration(ProfilerInfo *) Line 199	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!ProfControlBlock::IterateProfilers(ProfilerCallbackType) Line 207	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!ProfControlBlock::DoProfilerCallback(ProfilerCallbackType) Line 295	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!ProfControlBlock::ModuleUnloadStarted(unsigned __int64) Line 691	C++	Symbols loaded.
 	coreclr.dll!Module::Destruct() Line 662	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!ClassLoader::FreeModules() Line 1884	C++	Symbols loaded.
 	coreclr.dll!ClassLoader::~ClassLoader() Line 1946	C++	Symbols loaded.
 	coreclr.dll!Assembly::Terminate(int) Line 311	C++	Symbols loaded.
 	coreclr.dll!Assembly::~Assembly() Line 244	C++	Symbols loaded.
 	coreclr.dll!Assembly::`scalar deleting destructor'(unsigned int __flags)	C++	Symbols loaded.
 	coreclr.dll!DomainAssembly::~DomainAssembly() Line 91	C++	Symbols loaded.
 	coreclr.dll!DomainAssembly::`scalar deleting destructor'(unsigned int __flags)	C++	Symbols loaded.
>	coreclr.dll!LoaderAllocator::GCLoaderAllocators(LoaderAllocator * pOriginalLoaderAllocator) Line 570	C++	Symbols loaded.
 	coreclr.dll!LoaderAllocator::Destroy(QCall::LoaderAllocatorHandle pLoaderAllocator) Line 702	C++	Symbols loaded.
 	coreclr.dll!LoaderAllocator_Destroy(QCall::LoaderAllocatorHandle pLoaderAllocator) Line 718	C++	Symbols loaded.
 	System.Private.CoreLib.dll!00007ffa9c4308f9()	Unknown	No symbols loaded.
 	coreclr.dll!FastCallFinalizeWorker() Line 26	Unknown	Symbols loaded.
 	coreclr.dll!MethodTable::CallFinalizer(Object * obj) Line 4908	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!CallFinalizer(Object *) Line 75	C++	Symbols loaded.
 	coreclr.dll!FinalizerThread::FinalizeAllObjects() Line 110	C++	Symbols loaded.
 	coreclr.dll!FinalizerThread::FinalizerThreadWorker(void * args) Line 354	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!ManagedThreadBase_DispatchInner(ManagedThreadCallState *) Line 7222	C++	Symbols loaded.
 	coreclr.dll!ManagedThreadBase_DispatchMiddle(ManagedThreadCallState * pCallState) Line 7266	C++	Symbols loaded.
 	coreclr.dll!ManagedThreadBase_DispatchOuter(ManagedThreadCallState * pCallState) Line 7425	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!ManagedThreadBase_NoADTransition(void(*)(void *)) Line 7494	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!ManagedThreadBase::FinalizerBase(void(*)(void *)) Line 7513	C++	Symbols loaded.
 	coreclr.dll!FinalizerThread::FinalizerThreadStart(void * args) Line 403	C++	Symbols loaded.
 	kernel32.dll!BaseThreadInitThunk()	Unknown	Symbols loaded.
 	ntdll.dll!RtlUserThreadStart()	Unknown	Symbols loaded.

Thread exit with threadlocal cleanup

>	coreclr.dll!LoaderAllocator::SetHandleValue(unsigned __int64 handle, Object *) Line 992	C++	Symbols loaded.
 	coreclr.dll!LoaderAllocator::FreeHandle(unsigned __int64 handle) Line 884	C++	Symbols loaded.
 	coreclr.dll!ThreadLocalBlock::FreeTLM(unsigned __int64 i, int isThreadShuttingdown) Line 59	C++	Symbols loaded.
 	coreclr.dll!ThreadLocalBlock::FreeTable() Line 93	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!Thread::DeleteThreadStaticData() Line 7704	C++	Symbols loaded.
 	coreclr.dll!Thread::OnThreadTerminate(int holdingLock) Line 2956	C++	Symbols loaded.
 	coreclr.dll!DestroyThread(Thread * th) Line 924	C++	Symbols loaded.
 	coreclr.dll!ThreadNative::KickOffThread(void * pass) Line 239	C++	Symbols loaded.
 	kernel32.dll!BaseThreadInitThunk()	Unknown	Symbols loaded.
 	ntdll.dll!RtlUserThreadStart()	Unknown	Symbols loaded.

and exception thrown with the following details

Exception thrown: read access violation.
**loaderAllocator** was nullptr.
devenv_iJFpiWBvYe

loaderAllocator value from the LOADERALLOCATORREF loaderAllocator = (LOADERALLOCATORREF)ObjectFromHandle(m_hLoaderAllocatorObjectHandle); call is NULL which makes sense, because finalizer thread just disposed data for some of the same LoaderAllocator assemblies.

Note: enabling COR_PRF_MONITOR_MODULE_LOADS CLR profiler flag increases chances for the race condition as it slows down a bit complete loader destruction.

Changes:

I've looked at the following options as a solution to the problem:

  • Null check for loaderAllocator
  • Wait for GC/Finalizers done before killing thread
  • Skip cleaning up thread locals for unloaded loaders

I think skipping unloaded loaded is the best option wrt performance/correctness considerations as loader deletion will cleanup all internal data, we won't access memory which may be already returned to pool/system and we won't lock other threads.

@alexey-zakharov alexey-zakharov marked this pull request as ready for review March 15, 2024 16:05
@alexey-zakharov alexey-zakharov requested a review from joncham March 15, 2024 16:05
@alexey-zakharov alexey-zakharov self-assigned this Mar 18, 2024
@alexey-zakharov alexey-zakharov merged commit 3444d84 into unity-main Mar 18, 2024
@alexey-zakharov alexey-zakharov deleted the fix-threadlocal-free branch March 18, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants