[CrashReporter] Enable InProc CrashReporter for crashing GC threads - #131821
[CrashReporter] Enable InProc CrashReporter for crashing GC threads#131821mdh1418 wants to merge 1 commit into
Conversation
Reuse an existing suspension only when the crashing thread owns it, and allow workstation background GC to perform reporter-owned suspend and resume while preserving Server GC deadlock safeguards. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: aa976a9e-b875-4524-82e1-1485d03d14fa
|
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. |
There was a problem hiding this comment.
Pull request overview
This PR updates CoreCLR’s in-proc crash-report stack walker to allow managed thread enumeration in additional GC/suspension scenarios by tracking whether a runtime suspension is (a) unavailable, (b) already established and owned by the crashing thread, or (c) created by the crash reporter.
Changes:
- Introduces
CrashReportSuspensionOwnershipto distinguish between an unusable suspension, a usable existing suspension, and a reporter-created suspension. - Reuses an existing completed suspension only when the crashing thread owns the ThreadStore lock; otherwise avoids enumerating other managed threads.
- Resumes the runtime only when the crash reporter itself performed the suspension.
| // Do not wait for the ThreadStore lock while another suspension is starting or | ||
| // ending. In particular, a Server GC coordinator may hold the lock while waiting | ||
| // for a crashing parallel worker at a GC join. A fatal error already recorded on | ||
| // a GC thread also means the interrupted GC may never complete. | ||
| // |
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
| { | ||
| return crashThreadOwnsSuspension | ||
| ? CrashReportSuspensionOwnership::Existing | ||
| : CrashReportSuspensionOwnership::Unavailable; |
There was a problem hiding this comment.
If I am reading this correctly, we will take CrashReportSuspensionOwnership::Unavailable; path when encounter a crash on a server GC thread. Is it the behavior we want?
There was a problem hiding this comment.
The scenario I was imagining is in Server GC, there is a dedicated heap 0 thread that coordinates suspension, and the crash occurs in a Parallel Server GC worker. I was thinking that since the heap 0 thread controls SuspendEE/RestartEE that it wouldn't be safe to stackwalk other threads if the heap 0 thread can resume independently.
If the crashing thread is the one that owns Suspension, then we are able to walk suspended threads callstacks given it would be the one to RestartEE.
Does that sound right?
Previously, the InProc CrashReporter skipped suspending and enumerating other managed threads during any GC scenario. However, in select GC scenarios, the thread running the reporter either owns the existing thread suspension or can safely establish its own suspension and walk managed call stacks.
This change tracks suspension ownership so the reporter can: