VM: Possible Lock Ordering Bug in Destructor for VirtualCallStubManager
- Dominant language
- C#
- Stars
- 18.3k
- Forks
- 5.6k
- PR merge metrics
- PR metrics pending
Description
### Description
I encountered a VM assertion failure while doing a run of Fuzzlyn with new changes from https://github.com/jakobbotsch/Fuzzlyn/pull/23. Fuzzlyn uses AssemblyLoadContext to load generated assemblies from a client into its execution server and unloads the AssemblyLoadContext every ~100 iterations. With this new PR, Fuzzlyn now uses separate process pools for the base/diff programs, so each iteration is split in half, meaning that those unload events happen effectively **twice as often**. That seems like a possible explanation for why this surfaced.
## Copilot Analysis:
> [!NOTE]
> The following was generated with Copilot assistance.
Root cause: this is a CoreCLR lock-scope bug introduced by dotnet/runtime#128868.
VirtualCallStubManager::~VirtualCallStubManager() acquires CrstStubDispatchCache while unlinking entries. Because #ifdef does not create a C++ scope, the CrstHolder remains alive for the rest of the destructor. The destructor subsequently deletes executable loader heaps, which acquire CrstExecutableAllocatorLock . Both locks are level 0, producing the exact assertion:
Can't take level 0 lock CrstExecutableAllocatorLock
because you already holding level 0 lock CrstStubDispatchCache
The surgical runtime fix is to scope the cache lock around only the unlink loop:
```c++
{
#ifdef CHAIN_LOOKUP
CrstHolder lh(g_resolveCache->GetWriteLock());
#endif
DispatchCache::Iterator it(g_resolveCache);
while (it.IsValid())
{
while (it.IsValid() && cache_entry_rangeList.IsInRange((TADDR)it.Entry()))
{
it.UnlinkEntry();
}
it.Next();
}
} // Release CrstStubDispatchCache before deleting executable heaps.
```
### Reproduction Steps
Found with:
`$ dotnet Fuzzlyn.dll --host "$env:CORE_ROOT\corerun.exe" --num-programs 10000 --interpreter-vs-jit`
### Expected behavior
No assertion.
### Actual behavior
Assert:
```
00:01:06 elapsed, 100/10000 programs generated, 0 examples found
00:02:21 elapsed, 200/10000 programs generated, 0 examples found
00:03:41 elapsed, 300/10000 programs generated, 0 examples found
00:04:50 elapsed, 400/10000 programs generated, 0 examples found
00:06:16 elapsed, 500/10000 programs generated, 0 examples found
Found 155.0 KiB example with seed 5627977783545531455 that hits error
[Diff]
Assert failure(PID 38224 [0x00009550], Thread: 1748 [0x06d4]): Consistency check failed: Crst Level violation: Can't take level 0 lock CrstExecutableAllocatorLock because you already holding level 0 lock CrstStubDispatchCache
FAILED: false
CORECLR! CHECK::Trigger + 0x21A (0x00007ffb`771ee8ea)
CORECLR! CrstBase::IsSafeToTake + 0x38A (0x00007ffb`774ac80a)
CORECLR! CrstBase::Enter + 0x10D (0x00007ffb`774ac0ad)
CORECLR! ClrEnterCriticalSection + 0x13E (0x00007ffb`777b8f3e)
CORECLR! ExecutableAllocator::ReleaseWorker + 0x66 (0x00007ffb`7722fc66)
CORECLR! UnlockedLoaderHeap::~UnlockedLoaderHeap + 0x143 (0x00007ffb`771f1d93)
CORECLR! LoaderHeap::`scalar deleting destructor' + 0x43 (0x00007ffb`775a02f3)
CORECLR! VirtualCallStubManager::~VirtualCallStubManager + 0x2F8 (0x00007ffb`776b1cd8)
CORECLR! VirtualCallStubManager::`scalar deleting destructor' + 0x14 (0x00007ffb`775a0374)
CORECLR! LoaderAllocator::GCLoaderAllocators + 0x5B2 (0x00007ffb`775a6a52)
File: C:\Users\adamperlin\dev\runtime\src\coreclr\vm\crst.cpp:684
Image: C:\Users\adamperlin\dev\runtime\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\corerun.exe
```
### Regression?
_No response_
### Known Workarounds
_No response_
### Configuration
Revision:
`Main`: c4eee2b76e574b0dd6cfe3387220a905ba69aca6
OS: Windows
Architecture: x64
### Other information
_No response_
Contributor guide
Research direction
Start at VirtualCallStubManager::~VirtualCallStubManager and the CrstHolder around the DispatchCache unlink loop; crst.cpp:684 is the assertion site. Reproduce with the supplied Fuzzlyn command and inspect whether CrstStubDispatchCache remains held while executable loader heaps are deleted. Done means the lock-level assertion no longer occurs during repeated AssemblyLoadContext unloads.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 62/100