Skip to content

Fix intermittent teardown crash in the cDAC GC stress framework#130526

Open
max-charlamb wants to merge 2 commits into
dotnet:mainfrom
max-charlamb:cdac-stress-shutdown-fix
Open

Fix intermittent teardown crash in the cDAC GC stress framework#130526
max-charlamb wants to merge 2 commits into
dotnet:mainfrom
max-charlamb:cdac-stress-shutdown-fix

Conversation

@max-charlamb

@max-charlamb max-charlamb commented Jul 10, 2026

Copy link
Copy Markdown
Member

Summary

Fixes an intermittent crash at process teardown in the cDAC GC stress framework, seen on arm/arm64 as exit 139 (SIGSEGV). Each affected run completes all verifications and prints its summary (e.g. MultiThread ARGITER 84 pass / 0 fail), then crashes during shutdown -- so the fault is in teardown, not in the verification logic.

Split out of #130447 so it can land independently.

Root cause

CdacStressPolicy::Shutdown runs from EEShutDownHelper before the finalizer/thread rendezvous, so background threads can still be allocating (the MultiThread debuggee spins up worker threads). Such a thread can pass the IsEnabled() gate and enter VerifyAtStressPoint, which uses the cDAC reader and cached COM interfaces, exactly as Shutdown releases/frees them -- a use-after-release.

Fix

Shutdown holds s_cdacLock across the whole teardown and clears s_initialized under it. VerifyAtStressPoint re-checks s_initialized under the same lock and bails, so no verification runs against state being torn down. cdac_reader_free is still called as before.

Testing

The crash is arm/arm64-only (does not reproduce on x64), so only CI can validate it. An x64 MultiThread stress soak (80 runs) passes with summaries intact and this change does not alter the x64 behavior.

Note

This PR description and change were produced with the assistance of GitHub Copilot.

Copilot AI review requested due to automatic review settings July 10, 2026 19:52
max-charlamb pushed a commit to max-charlamb/runtime that referenced this pull request Jul 10, 2026
The shutdown/teardown crash fix in src/coreclr/vm/cdacstress.cpp is being
tracked separately in dotnet#130526 so it can land independently of the cDAC
arg-GCRefMap changes in this PR. This restores cdacstress.cpp to its
upstream state here.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: dc4ebcc7-53f1-47e9-a84e-830b48bd07b2

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

This PR adjusts the CoreCLR cDAC GC stress shutdown path to avoid teardown-time crashes by disabling the stress trigger gate earlier, draining in-flight verifications under s_cdacLock, and intentionally leaking the cDAC reader handle/module at process exit rather than freeing/unloading them during EE shutdown.

Changes:

  • Disable stress triggers (s_initialized / s_cdacStressLevel) before teardown, then acquire s_cdacLock to ensure any in-flight VerifyAtStressPoint completes before releasing shared state.
  • Preserve accurate summary reporting by snapshotting which sub-checks were enabled before the shutdown gate is disabled.
  • Stop calling cdac_reader_free during shutdown (and avoid unloading the cDAC module), to prevent teardown races with background stress-hook threads.

Comment thread src/coreclr/vm/cdacstress.cpp Outdated
@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.

Copilot AI review requested due to automatic review settings July 10, 2026 20:23
@max-charlamb max-charlamb force-pushed the cdac-stress-shutdown-fix branch from 0f8ae6f to 4d3adf9 Compare July 10, 2026 20:23

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

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

Comment thread src/coreclr/vm/cdacstress.cpp
Comment thread src/coreclr/vm/cdacstress.cpp Outdated
Comment thread src/coreclr/vm/cdacstress.cpp
Copilot AI review requested due to automatic review settings July 10, 2026 21:25
@max-charlamb max-charlamb force-pushed the cdac-stress-shutdown-fix branch from 4d3adf9 to 958543d Compare July 10, 2026 21:25

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

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

Comment thread src/coreclr/vm/cdacstress.cpp Outdated
Comment thread src/coreclr/vm/cdacstress.cpp Outdated
Copilot AI review requested due to automatic review settings July 10, 2026 21:57
@max-charlamb max-charlamb force-pushed the cdac-stress-shutdown-fix branch from 958543d to 9d1ffd5 Compare July 10, 2026 21:57

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

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

Comment thread src/coreclr/vm/cdacstress.cpp Outdated
Comment thread src/coreclr/vm/cdacstress.cpp
Copilot AI review requested due to automatic review settings July 10, 2026 22:04
@max-charlamb max-charlamb force-pushed the cdac-stress-shutdown-fix branch from 9d1ffd5 to 961fd13 Compare July 10, 2026 22:04

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

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

Comment thread src/coreclr/vm/cdacstress.cpp
Comment thread src/coreclr/vm/cdacstress.cpp
The cDAC stress harness crashed intermittently at process teardown on
arm/arm64: each run completed all verifications and printed its summary
(e.g. MultiThread ARGITER "84 pass / 0 fail"), then the process exited 139
(SIGSEGV). The uploaded per-debuggee logs show the crash is after
verification finishes, during shutdown.

CdacStressPolicy::Shutdown runs from EEShutDownHelper (g_fEEShutDown =
ShutDown_Start is set immediately before it, and the finalizer/thread
rendezvous happens after), so background threads may still be allocating --
the MultiThread debuggee spins up worker threads. Such a thread can pass the
IsEnabled() gate and enter VerifyAtStressPoint, which uses the cDAC reader
and cached COM interfaces, exactly as Shutdown releases/frees them.

Fix: Shutdown holds s_cdacLock across the entire teardown and clears
s_initialized under it (the reader is still freed via cdac_reader_free as
before). This drains any in-flight VerifyAtStressPoint and blocks new ones,
which re-check s_initialized under the same lock and bail, so none runs
against released state. s_cdacLock's acquire/release ordering publishes the
transition, so no volatile accesses are needed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: dc4ebcc7-53f1-47e9-a84e-830b48bd07b2
Copilot AI review requested due to automatic review settings July 10, 2026 22:11
@max-charlamb max-charlamb force-pushed the cdac-stress-shutdown-fix branch from 961fd13 to a554e1a Compare July 10, 2026 22:11

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread src/coreclr/vm/cdacstress.cpp
@max-charlamb max-charlamb marked this pull request as ready for review July 10, 2026 22:20
@max-charlamb max-charlamb requested a review from rcj1 July 10, 2026 22:20
@max-charlamb

Copy link
Copy Markdown
Member Author

/azp run runtime-diagnostics

Note

This comment was generated with the assistance of GitHub Copilot.

@azure-pipelines

Copy link
Copy Markdown
No pipelines are associated with this pull request.

src/coreclr/vm/cdacstress.cpp is the native cDAC GC stress harness that the
runtime-diagnostics CdacStressTest legs exercise, but it was not in the
pipeline's pr path filter, so changes to it (like the shutdown fix in this
PR) did not trigger the very legs that validate them. Add it to the include
list.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: dc4ebcc7-53f1-47e9-a84e-830b48bd07b2
Copilot AI review requested due to automatic review settings July 10, 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

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

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.

4 participants