[release/11.0] Fix VSD cache lock ordering during unload (backport #133213) - #134159
JulieLeeMSFT merged 1 commit into
Conversation
## Summary - release the virtual stub dispatch cache lock before deleting loader heaps that acquire the executable allocator lock - add a collectible `AssemblyLoadContext` regression test that grows the VSD cache entry heap and exercises the lock-order failure Fixes dotnet#132982 ## Testing - `build.cmd clr -rc checked` - `src\tests\build.cmd -Test Regressions\coreclr\GitHub_132982\test132982.csproj x64 checked -Priority 1` - confirmed the regression test triggers the `CrstStubDispatchCache` / `CrstExecutableAllocatorLock` level violation without the fix and passes with the fix > [!NOTE] > This pull request description was generated with GitHub Copilot. --------- Copilot-Session: dafdc384-5b50-4f91-a024-00a1dafbc0d3
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @agocke |
There was a problem hiding this comment.
🟡 Changes recommended
Add the required process-isolation property to the regression test project.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Backports a fix for VSD cache lock ordering during collectible AssemblyLoadContext unloading.
Changes:
- Narrows the cache lock scope before loader-heap cleanup.
- Adds a regression test exercising collectible unloading and VSD cache growth.
- Configures CoreCLR test compatibility settings.
File summaries
| File | Reviewed changes |
|---|---|
src/tests/Regressions/coreclr/GitHub_132982/test132982.csproj |
Registers the regression test; requires process-isolation configuration. |
src/tests/Regressions/coreclr/GitHub_132982/test132982.cs |
Tests collectible AssemblyLoadContext unloading and VSD cache growth. |
src/coreclr/vm/virtualcallstub.cpp |
Releases the cache lock before loader-heap cleanup. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
|
Approved pending you resolve copilot feedback, get code review and check CI failures. |
|
Hi, the code complete date for RC2 is Monday 21st September. Make sure to merge this PR on that date at the latest, or it won't make it into that release. As a reminder, if this is a product change, you also need Tactics approval before merging this PR (test-only or infra-only changes don't require Tactics approval). |
|
/ba-g no failures but build analysis is red. Known issue for broken build analysis. |
.NET 11 fix is M1 approval until RC2 snap. After that, .NET 11 fix will need tactics approval. |
Backport of #133213 to release/11.0
Customer Impact
The PR fixes a lock issue which occurs in the VirtualCallStubManager destructor when cleaning up an AssemblyLoadContext. It is a regression for .NET 11 that originates in #128868 where a new lock aquisition was added for
g_resolveCache.Regression
Testing
A regression test was added that successfully reproduced the issue, which is now passing with the fix.
Risk
Low.