Skip to content

[cDAC] Add ThreadState.ReportDead to thread state enum#126484

Open
max-charlamb wants to merge 1 commit intodotnet:mainfrom
max-charlamb:fix/thread-reportdead
Open

[cDAC] Add ThreadState.ReportDead to thread state enum#126484
max-charlamb wants to merge 1 commit intodotnet:mainfrom
max-charlamb:fix/thread-reportdead

Conversation

@max-charlamb
Copy link
Copy Markdown
Member

@max-charlamb max-charlamb commented Apr 2, 2026

Summary

Add ThreadState.ReportDead (0x00010000) to the cDAC thread state enum.

The native DAC's GetSnapshotState() synthesizes TS_Dead from TS_ReportDead, which is set during WaitForOtherThreads. Without this flag, cDAC consumers checking for dead threads miss threads that are in the ReportDead state but don't yet have TS_Dead set.

This was discovered via internal stepping tests where IsThreadMarkedDead returned FALSE for a thread the native DAC reported as dead.

Changes

  • Add ReportDead = 0x00010000 to ThreadState enum in IThread.cs
  • Add unit test verifying the flag is preserved through GetThreadData

Note

This PR was generated with the assistance of GitHub Copilot.

Copy link
Copy Markdown
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

Adds the missing ThreadState.ReportDead flag to the cDAC thread state contract surface so managed consumers can observe this runtime state (which the native DAC may later treat as dead via snapshot state synthesis).

Changes:

  • Add ThreadState.ReportDead = 0x00010000 to IThread.cs.
  • Add a unit test that writes ReportDead into the mock thread’s State field and verifies it round-trips through GetThreadData.

Reviewed changes

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

File Description
src/native/managed/cdac/tests/ThreadTests.cs Adds a new theory verifying ReportDead is preserved through GetThreadData.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IThread.cs Extends ThreadState with the ReportDead flag and an explanatory comment.

The native DAC's GetSnapshotState() synthesizes TS_Dead from
TS_ReportDead (0x00010000), set during WaitForOtherThreads.
Without this flag, IsThreadMarkedDead and EnumerateThreads miss
threads in the ReportDead state.

- Add ReportDead to ThreadState enum in IThread.cs
- Fix IsThreadMarkedDead to check both Dead and ReportDead
- Fix EnumerateThreads to skip ReportDead threads
- Update Thread contract docs (Thread.md)
- Add unit test verifying the flag is preserved through GetThreadData

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
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

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

Comment on lines 340 to 343
Contracts.ThreadData threadData = threadContract.GetThreadData(currentThread);
// Match native: skip dead and unstarted threads
if ((threadData.State & (Contracts.ThreadState.Dead | Contracts.ThreadState.Unstarted)) == 0)
// Match native: skip dead and unstarted threads (ReportDead synthesizes Dead)
if ((threadData.State & (Contracts.ThreadState.Dead | Contracts.ThreadState.ReportDead | Contracts.ThreadState.Unstarted)) == 0)
{
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

With ReportDead now treated equivalently to Dead here, the existing dump-based tests that compute expectations using only ThreadState.Dead will no longer reflect the same semantics and could fail (or miss coverage) if a dump contains ReportDead-only threads. Update/add tests to treat ReportDead as dead when validating EnumerateThreads behavior.

Copilot uses AI. Check for mistakes.
Comment on lines 391 to 395
{
Contracts.ThreadData threadData = _target.Contracts.Thread.GetThreadData(new TargetPointer(vmThread));
*pResult = (threadData.State & Contracts.ThreadState.Dead) != 0 ? Interop.BOOL.TRUE : Interop.BOOL.FALSE;
// Match native GetSnapshotState: TS_ReportDead synthesizes TS_Dead
*pResult = (threadData.State & (Contracts.ThreadState.Dead | Contracts.ThreadState.ReportDead)) != 0 ? Interop.BOOL.TRUE : Interop.BOOL.FALSE;
}
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

IsThreadMarkedDead now returns TRUE for TS_ReportDead as well as Dead. Any cross-validation that compares this result to only the Dead flag from ThreadData.State will be inconsistent. Tests should be adjusted/extended to validate the new intended behavior (ReportDead implies dead for this API).

Copilot uses AI. Check for mistakes.
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.

2 participants