dotnet / dotnet/runtime

VM: Possible Lock Ordering Bug in Destructor for VirtualCallStubManager

Open
#132,982 1 comment 0 reactions 0 assignees View on GitHub
area-VM-coreclr
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.