Repository navigation
Conversation
Make the non-inlined PollGCWorker helper binder-visible and bind it before managed execution, allowing stack walks to recognize polling helpers without loading methods during suspension. Omit only leading PollGC and PollGCWorker frames in the shared EventPipe stack walker, preserving caller frames, samples, and classification. Add raw-stack regression coverage using JIT and rundown method ranges, including caller retention and ordinary event stacks. Fixes dotnet#135050
|
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. |
steveisok
approved these changes
Oct 23, 2026
Member
|
/ba-g Known issues |
steveisok
pushed a commit
that referenced
this pull request
Oct 25, 2026
> **TL;DR:** `SampleProfilerSampleType` intermittently throws `OverflowException` on linux-arm Checked. This PR changes the test's stack-address conversion to prevent that overflow. The fix still needs validation in linux-arm CI. It also needs backports to release/11.0 and release/10.0. `SampledStacksOmitGCPollHelpers`, added in #135219, fails intermittently on linux-arm Checked. It throws `OverflowException` from `RawEventPipeStack.Read`. In build 1647637, the same test passed on windows-x86, linux-x64 and linux-arm64. Fixes #136197 ## Cause TraceEvent 3.1.28 fills `DataPtr` for a stack attachment with `(ulong)(eventData.StackBytes - 8)`, where `StackBytes` is an `IntPtr`. That calls `IntPtr`'s explicit conversion to `long`, which sign-extends in a 32-bit process. An attachment at or above `0x80000000` reads back as `0xFFFFFFFF_xxxxxxxx`, and the test's `checked((nuint)address)` throws. Whether the buffer lands that high depends on the process's memory layout. That fits the same Helix image both passing and failing, though the failing runs don't log the pointer value. I fetched run details for 33 report rows in #136197; all 33 are linux-arm Checked. Two other rows returned no details, and one row was added after I collected the details. I also scanned the scheduled main outerloop and jitstress builds from October 18 to 21. The first with the overflow is 1640173, queued about 49 minutes after #135219 merged. The linux-arm tracing work items in the six builds before it show no `OverflowException`. ## Change Convert with `unchecked((nint)address)`, which keeps only the native-width bits. On 64-bit platforms the result is unchanged. ## Validation I couldn't run the test on linux-arm. A small program on x64 applies the 32-bit forms of both conversions to a sign-extended value, `0xFFFFFFFFB6A01000`. With the 32-bit forms, the old conversion throws `OverflowException` and the new one returns `0xB6A01000`. The program also converts `0x00007F0012345678` with the 64-bit forms, and both return `0x7F0012345678`. The linux-arm legs of this PR's CI are the real check, and since the failure is intermittent, one green run shows little. ## Backports #135219 was backported to release/11.0 (#136077) and release/10.0 (#136091), so this fix needs the same backports. cc @lateralusX > [!NOTE] > This change was prepared with AI assistance (Anthropic Claude, checked by OpenAI Codex) at my request. I reviewed the change and the evidence before opening.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes Issue #135050
main PR #135219
Description
Manually adapted backport of #135219 to
release/10.0, corresponding to the .NET 11 backport in #136077.EventPipe sampling suspension can cause threads to enter the managed GC polling helpers, which then appear at the top of captured stacks and obscure the actual workload.
Make
PollGCWorkerbinder-visible and bind it during runtime startup, allowing the EventPipe stack walker to omit leadingPollGCandPollGCWorkerframes without loading methods during suspension. Preserve caller frames, samples, and sample classification.The backport uses
Thread.CoreCLR.csand the feature guards appropriate to .NET 10, preserving its existing null-control-PC handling. Regression coverage is added to the existingGCEventstest project because .NET 10 does not have the newerSampleProfilerSampleTypetest.Customer Impact
CPU-sampling profiles can attribute almost all samples to
PollGCWorkerinstead of the application workload, making performance investigations misleading. This is a profiling artifact, not evidence of equivalent CPU usage or GC pressure.Regression
Yes. The issue was reported as a regression from .NET 9 to .NET 10 after GC polling moved into managed helpers. The regression was reproduced locally on the unfixed .NET 10 runtime.
Testing
Validated locally on Windows x64 with Checked CoreCLR and Release libraries.
Build commands, all completed successfully with the local-only build prerequisites described below:
build.cmd clr+libs+host -rc Checked -lc Release -hc Release src\tests\build.cmd -Test tracing\eventpipe\eventsvalidation\GCEvents.csproj x64 Checked -Priority 1 -SkipNative /p:LibrariesConfiguration=Release /p:HostConfiguration=Release build.cmd clr -rc Checked -lc Release -hc Release src\tests\build.cmd -GenerateLayoutOnly x64 Checked /p:LibrariesConfiguration=Release /p:HostConfiguration=ReleaseTest commands:
Also ran
GCEvents.dllwithDOTNET_TieredCompilation=0: both tests passed with exit code 100, checking 9,943 sample stacks with zero lost events.The regression checks raw sample stacks, retention of workload frames in managed samples, and retention of workload frames in ordinary event stacks.
Other platforms and configurations have not been validated for this backport.
Risk
Low. The change is narrowly scoped to identifying GC polling helpers and omitting them only while they are leading frames in an EventPipe stack. It does not change GC polling or suspension semantics, discard samples, or change sample classification.
The worker is bound before managed execution, avoiding method loading during stack walks. Existing handling of other frames is preserved. Local testing confirmed the original failure and verified both the regression and existing GC-events coverage after the fix, with default tiering and tiering disabled.
Resolves #135050