Skip to content

Backport/pr 129021 to release/10.0#129142

Open
davidwrighton wants to merge 8 commits into
dotnet:release/10.0from
davidwrighton:backport/pr-129021-to-release/10.0
Open

Backport/pr 129021 to release/10.0#129142
davidwrighton wants to merge 8 commits into
dotnet:release/10.0from
davidwrighton:backport/pr-129021-to-release/10.0

Conversation

@davidwrighton

Copy link
Copy Markdown
Member

Backport of #129021 to release/10.0

Fixes #128401

/cc @leculver

Customer Impact

  • Customer reported
  • Found internally

This was reported by customers which found that a service in production would deadlock occasionally when perfmap generation was enabled.

Regression

  • Yes
  • No

This was introduced in #113943

Testing

Fix was verified through analysis, and the fix also includes an escape hatch environment variable (DOTNET_PerfMapGranularity=4) which will disable the problematic code entirely.

Risk

  1. This fix is only applicable to customers which use perfmaps
  2. The fix not only includes code changes which fix the deadlock problem from happening, but also includes a change to add a config switch to avoid the problematic code entirely while keeping the perfmap generation for most scenarios which are interesting to customers.

Risks:
This fix adjusts a lock so that it will not switch to preemptive mode, and we will run some file I/O while under that lock. This may cause GC latency spikes on some customer workloads. This issue is mitigated by the new switch which can be used to avoid the problem in its entirety at the cost of slightly less accurate profile data for a limited set of scenarios.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • For .NET 8 and .NET 9: The PR target branch is release/X.0-staging, not release/X.0.
  • For .NET 10+: The PR target branch is release/X.0 (no -staging suffix).

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

Copilot AI review requested due to automatic review settings June 8, 2026 23:01
@davidwrighton davidwrighton requested review from hoyosjs and jkotas June 8, 2026 23:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Backport of the .NET 10 CoreCLR PerfMap deadlock fix to release/10.0, addressing a lock-ordering / GC-mode-toggle deadlock when PerfMap stub logging runs on paths that hold CRST_UNSAFE_ANYMODE locks (notably virtual-call-stub / code fragment heap allocation paths). The change also introduces a configuration escape hatch to disable stub logging while keeping PerfMap generation available for other symbol types.

Changes:

  • Make the PerfMap lock (PerfMap::s_csPerfMap) CRST_UNSAFE_ANYMODE to prevent cooperative↔preemptive GC-mode toggling while under other UNSAFE_ANYMODE locks.
  • Add a PerfMapStubGranularity bit to disable stub logging (value 4), providing an escape hatch for affected customers.
  • Adjust virtual call stub generation paths to optionally switch GC mode around stub generation and to reinitialize bucket-table probers after potential GC-mode transitions.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/coreclr/vm/virtualcallstub.cpp Wraps stub generation with conditional GC-mode switching when PerfMap is enabled, and refreshes probers after potential invalidation; removes FEATURE_PERFMAP guards around stub logging calls.
src/coreclr/vm/perfmap.h Adds a no-FEATURE_PERFMAP stub PerfMap definition and introduces s_LogStubs in the FEATURE_PERFMAP path.
src/coreclr/vm/perfmap.cpp Switches PerfMap crst to CRST_UNSAFE_ANYMODE, adds s_LogStubs config handling, and gates stub logging on the new switch.
src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h Updates PerfMap enable path contracts for diagnostics-server activation.
src/coreclr/inc/clrconfigvalues.h Updates the PerfMapStubGranularity description to document the new “disable stub logging” bit.

Comment thread src/coreclr/vm/perfmap.h
Comment on lines +27 to +34
static bool IsEnabled()
{
#ifdef DEBUG
return true;
#else
return false;
#endif
}
Comment on lines +1131 to +1139
bool reenteredCooperativeGCMode = PerfMap::IsEnabled();
{
GCX_MAYBE_PREEMP(reenteredCooperativeGCMode);
pLookupHolder = GenerateLookupStub(addrOfResolver, token.To_SIZE_T());
}

if (reenteredCooperativeGCMode)
{
// The prober may have been invalidated by reentering cooperative GC mode, reset it
RETAIL_CONFIG_DWORD_INFO(EXTERNAL_PerfMapIgnoreSignal, W("PerfMapIgnoreSignal"), 0, "When perf map is enabled, this option will configure the specified signal to be accepted and ignored as a marker in the perf logs. It is disabled by default")
RETAIL_CONFIG_DWORD_INFO(EXTERNAL_PerfMapShowOptimizationTiers, W("PerfMapShowOptimizationTiers"), 1, "Shows optimization tiers in the perf map for methods, as part of the symbol name. Useful for seeing separate stack frames for different optimization tiers of each method.")
RETAIL_CONFIG_DWORD_INFO(EXTERNAL_PerfMapStubGranularity, W("PerfMapStubGranularity"), 0, "Report stubs with varying amounts of granularity (low bit being zero indicates attempt to group all stubs of a type together) (second lowest bit being non-zero records stubs at individual allocation sites, which is more expensive, but also more accurate).")
RETAIL_CONFIG_DWORD_INFO(EXTERNAL_PerfMapStubGranularity, W("PerfMapStubGranularity"), 0, "Report stubs with varying amounts of granularity (low bit being zero indicates attempt to group all stubs of a type together) (second lowest bit being non-zero records stubs at individual allocation sites, which is more expensive, but also more accurate) (third lowest bit disables stub logging).")
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.

3 participants