Skip to content

[release/10.0] Fix SampleType field in ThreadSample event from Sample Profiler#124084

Merged
steveisok merged 1 commit intodotnet:release/10.0from
steveisok:backport-124019
Feb 6, 2026
Merged

[release/10.0] Fix SampleType field in ThreadSample event from Sample Profiler#124084
steveisok merged 1 commit intodotnet:release/10.0from
steveisok:backport-124019

Conversation

@steveisok
Copy link
Member

Backport of #124019 to release/10.0

Customer Impact

  • Customer reported
  • Found internally

The SampleType field in ThreadSample events from SampleProfiler was always reporting EP_SAMPLE_PROFILER_SAMPLE_TYPE_EXTERNAL even when threads were executing managed code. This causes incorrect profiling data for customers using diagnostics tools that rely on this event.

Fixes #123996

Regression

  • Yes
  • No

This regressed in .NET 9 when SuspendAllThreads was ported from NativeAOT to CoreCLR (commit 00a8973).

Testing

Manual

Risk

Low

The change is minimal and self-contained:

  • Adds a new thread state flag (TS_SuspensionTrapped) using a previously unused bit
  • Sets/clears the flag in RareDisablePreemptiveGC when threads voluntarily suspend
  • Uses the flag instead of the removed m_gcModeOnSuspension field for sample type detection
  • The change is pay-for-play: only threads that were actually executing managed code participate

…Profiler

Backport of dotnet#124019 to release/10.0

## Customer Impact

- [X] Customer reported
- [ ] Found internally

The SampleType field in ThreadSample events from SampleProfiler was always
reporting EP_SAMPLE_PROFILER_SAMPLE_TYPE_EXTERNAL even when threads were
executing managed code. This causes incorrect profiling data for customers
using diagnostics tools that rely on this event.

Fixes dotnet#123996

## Regression

- [X] Yes
- [ ] No

This regressed in .NET 9 when SuspendAllThreads was ported from NativeAOT
to CoreCLR (commit 00a8973).

## Testing

The fix was verified by building CoreCLR with Debug/Checked configuration
which validates the asmconstants offsets via compile-time assertions.

## Risk

Low

The change is minimal and self-contained:
- Adds a new thread state flag (TS_SuspensionTrapped) using a previously unused bit
- Sets/clears the flag in RareDisablePreemptiveGC when threads voluntarily suspend
- Uses the flag instead of the removed m_gcModeOnSuspension field for sample type detection
- The change is pay-for-play: only threads that were actually executing managed code participate
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

This PR is a backport of #124019 to the release/10.0 branch, fixing a regression in the SampleProfiler's ThreadSample event. In .NET 9+, the SampleType field incorrectly reported all samples as EP_SAMPLE_PROFILER_SAMPLE_TYPE_EXTERNAL instead of properly distinguishing managed code execution (EP_SAMPLE_PROFILER_SAMPLE_TYPE_MANAGED). The regression occurred when SuspendAllThreads was ported from NativeAOT to CoreCLR in commit 00a8973.

Changes:

  • Introduces a new thread state flag TS_SuspensionTrapped to track when threads voluntarily suspend while in managed (cooperative) mode
  • Removes the obsolete m_gcModeOnSuspension field and related methods from the Thread class
  • Updates assembly offset constants for ARM64 and AMD64 platforms to reflect the reduced Thread structure size

Reviewed changes

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

Show a summary per file
File Description
src/coreclr/vm/threads.h Adds TS_SuspensionTrapped flag using previously unused bit 0x00000002; removes m_gcModeOnSuspension field and its accessor methods
src/coreclr/vm/threadsuspend.cpp Sets TS_SuspensionTrapped when thread traps for suspension in cooperative mode; clears flag on resume
src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.cpp Uses HasThreadState(TS_SuspensionTrapped) to determine sample type instead of deleted GetGCModeOnSuspension()
src/coreclr/vm/arm64/asmconstants.h Adjusts m_pInterpThreadContext offset down by 8 bytes (0xb78→0xb70 Unix, 0xba0→0xb98 non-Unix) due to removed field
src/coreclr/vm/amd64/asmconstants.h Adjusts m_pInterpThreadContext offset down by 8 bytes (0xb50→0xb48 Unix, 0xba8→0xba0 non-Unix) due to removed field

@steveisok steveisok added Servicing-consider Issue for next servicing release review Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Feb 6, 2026
@steveisok steveisok requested review from jkotas and noahfalk February 6, 2026 18:13
@steveisok steveisok merged commit f3bc021 into dotnet:release/10.0 Feb 6, 2026
112 of 114 checks passed
@steveisok steveisok deleted the backport-124019 branch February 6, 2026 18:42
@jozkee jozkee added this to the 10.0.4 milestone Feb 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-VM-coreclr Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants