Skip to content

Fix CrstILStubGen/CrstLoaderAllocatorReferences lock level violation#125675

Merged
jkotas merged 2 commits intomainfrom
copilot/husky-cheetah
Mar 18, 2026
Merged

Fix CrstILStubGen/CrstLoaderAllocatorReferences lock level violation#125675
jkotas merged 2 commits intomainfrom
copilot/husky-cheetah

Conversation

Copy link
Contributor

Copilot AI commented Mar 17, 2026

CreateInteropILStub holds CrstILStubGen while type loading can reach EnsureInstantiationCrstLoaderAllocatorReferences, but this ordering was undeclared in CrstTypes.def. With CrstILStubGen at level 14 and CrstLoaderAllocatorReferences at level 17, attempting to acquire the latter while holding the former triggered a lock-level violation (e.g., during P/Invoke stub generation for types involving InlineArray<T> generics).

Changes

  • CrstTypes.def — Add ILStubGen to AcquiredAfter for LoaderAllocatorReferences, formally declaring that CrstLoaderAllocatorReferences may be acquired while CrstILStubGen is held.

  • crsttypes_generated.h — Regenerated via CrstTypeTool; CrstILStubGen level raised 14 → 18 (must exceed CrstLoaderAllocatorReferences at 17).

Original prompt

fix #125638 issue with these changes

[Chronological Review] 1. User requested investigation of bug https://github.com//issues/125638 - a Crst lock level violation in CoreCLR runtime 2. Read the GitHub issue (#125638) and revert PR (#125664), plus the original triggering PR (#125574) 3. Read issue comments to get the full stack trace from jkotas 4. Read PR #125664 diff to understand the revert (InlineArrays back to fixed buffers) 5. Launched background explore agent to find lock-related code paths 6. In parallel, grepped for CrstILStubGen, CrstLoaderAllocatorReferences, EnsureInstantiation 7. Examined loaderallocator.cpp (EnsureInstantiation, EnsureReference methods) 8. Examined ilstubcache.cpp (GetStubMethodDesc, CreateNewMethodDesc, CreateModuleIndependentSignature) 9. Examined dllimport.cpp (CreateInteropILStub - where CrstILStubGen is held via ListLockHolder) 10. Examined crst.cpp (IsSafeToTake - the lock level violation check logic) 11. Found CrstTypes.def - the configuration file for lock ordering 12. Read lock ordering declarations for ILStubGen and LoaderAllocatorReferences 13. Found Debug_CanTake() implementation - it actually Enter()/Leave() the lock 14. Created plan.md and todos 15. Implemented 3 changes: moved asserts in loaderallocator.cpp, updated CrstTypes.def, regenerated header 16. Built coreclr checked - first attempts had environment issues, clean rebuild succeeded 17. Ran self code review, verified all changes are correct 18. Marked all todos done

[Intent Mapping]

[Technical Inventory]

  • CoreCLR VM runtime (C++)
  • Crst (Critical Section) lock ordering system with levels
  • CrstTypes.def → CrstTypeTool → crsttypes_generated.h pipeline
  • CrstILStubGen: level 14→18, used by IL stub generation
  • CrstLoaderAllocatorReferences: level 17, used by LoaderAllocator
  • Debug_CanTake() actually calls Enter()/Leave() triggering level checks
  • ListLock/ListLockHolder pattern for ILStubGen lock
  • InlineArray generic types replacing fixed buffers in P/Invoke structs

[Code Archaeology]

  • src/coreclr/vm/loaderallocator.cpp: EnsureReference (line 261), EnsureInstantiation (line 287) - moved Debug_CanTake() asserts after IsCollectible() checks
  • src/coreclr/inc/CrstTypes.def: Added ILStubGen to AcquiredAfter for LoaderAllocatorReferences
  • src/coreclr/inc/crsttypes_generated.h: Regenerated, CrstILStubGen level 14→18
  • src/coreclr/vm/ilstubcache.cpp: GetStubMethodDesc, CreateNewMethodDesc, CreateModuleIndependentSignature
  • src/coreclr/vm/dllimport.cpp: CreateInteropILStub holds CrstILStubGen via ListLockHolder
  • src/coreclr/vm/siginfo.cpp: ConvertToInternalExactlyOne triggers type loading via GetTypeHandleThrowing
  • src/coreclr/vm/crst.cpp: IsSafeToTake (level violation check), crst.h: Debug_CanTake()
  • src/coreclr/vm/generics.cpp: CreateTypeHandleForNonCanonicalGenericInstantiation calls EnsureInstantiation

[Progress Assessment]

  • All tasks complete: assert relocation, CrstTypes.def update, header regeneration, build verification
  • Build succeeded: 0 errors, 0 warnings
  • Code review completed

[Context Validation]

  • Full root cause understood and documented
  • Fix implemented and verified
  • No PR created yet to the upstream repo

[Recent Commands Analysis]

  • Last significant commands: code-review skill execution, final verification of diffs and build output
  • Build succeeded with .\build.cmd clr.runtime -rc checked after clean rebuild
  • All 4 todos marked done
1. Conversation Overview: - Primary Objectives: User requested: "please investigate a bug in coreclr runtime system https://github.com//issues/125638 we know it was exposed by an unrelated change that replaced fixed-size buffers with InlineArrays, this PR reverts that change so we can unblock the CI: https://github.com//pull/125664 but we still want to fix the type system bug." - Session Context: Investigated a Crst lock level violation in CoreCLR where `CrstILStubGen` (level 14) was held while trying to acquire `CrstLoaderAllocatorReferences` (level 17). Traced full call chain, identified root cause as (a) `Debug_CanTake()` assert firing before early-return guards and (b) missing lock ordering declaration. Implemented fix across 3 files, built successfully. - User Intent Evolution: Single clear objective throughout - fix the underlying type system/lock ordering bug exposed by PR #125574.
  1. Technical Foundation:
    • CoreCLR VM Runtime: C++ codebase at C:\prj\runtime-main2\src\coreclr\vm
    • Crst Lock Ordering System: Levels defined in CrstTypes.def, processed by CrstTypeTool.cs → generates `crsttypes_gen...

Created from Copilot CLI via the copilot delegate command.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

Copilot AI changed the title [WIP] [DOTNET-125638] Fix Crst lock level violation in CoreCLR runtime Fix CrstILStubGen/CrstLoaderAllocatorReferences lock level violation Mar 17, 2026
Copilot AI requested a review from EgorBo March 17, 2026 20:39
…fix is sufficient

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot AI requested a review from jkotas March 17, 2026 20:44
@jkotas jkotas marked this pull request as ready for review March 17, 2026 20:53
Copilot AI review requested due to automatic review settings March 17, 2026 20:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates CoreCLR Crst (lock) hierarchy metadata to allow CrstLoaderAllocatorReferences to be taken after CrstILStubGen, and regenerates the derived lock-level map used for debug-time deadlock validation.

Changes:

  • Adds ILStubGen to the AcquiredAfter list for LoaderAllocatorReferences in CrstTypes.def.
  • Regenerates crsttypes_generated.h so CrstILStubGen’s computed level reflects the updated dependency graph.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/coreclr/inc/CrstTypes.def Adds ILStubGen as an allowed “acquired after” predecessor for LoaderAllocatorReferences to encode the intended lock ordering.
src/coreclr/inc/crsttypes_generated.h Updates the generated debug lock-level map to reflect the new ordering constraints (CrstILStubGen level change).

@jkotas jkotas merged commit 0389510 into main Mar 18, 2026
106 of 110 checks passed
@jkotas jkotas deleted the copilot/husky-cheetah branch March 18, 2026 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crst Level violation: Can't take level 17 lock CrstLoaderAllocatorReferences

4 participants