Skip to content

[CrashReporter] Enable InProc CrashReporter for crashing GC threads #131821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mdh1418 wants to merge 1 commit into
base: main
Choose a base branch
from
+37 −25
Open

[CrashReporter] Enable InProc CrashReporter for crashing GC threads #131821

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 37 additions & 25 deletions src/coreclr/vm/crashreportstackwalker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,40 +482,49 @@ CrashReportGetExceptionForThread(
return result;
}

// Suspend non-crashing managed threads via SuspendEE so their stacks
// can be walked from runtime-known safe points. SuspendEE acquires the
// thread store lock and waits for every other managed thread to reach a
// safe point (and for any in-progress GC to complete), so skip it when
// a known pre-condition would prevent forward progress:
// SuspendEE acquires the ThreadStore lock and retains it until the matching
// RestartEE. SysIsSuspended becomes true only after all managed threads have
// reached safe points. That completed suspension is usable only when the
// crashing thread holds the lock and therefore controls when RestartEE runs;
// otherwise, another thread could call RestartEE during enumeration and allow
// the managed threads being walked to leave their safe points.
//
// * g_fFatalErrorOccurredOnGCThread: GC thread faulted mid-GC, so GC
// will never finish and SuspendEE's GC wait would hang.
// * GCHeapUtilities::IsGCInProgress(): a GC is already running; if it
// is wedged (common in runtime-internal crashes) SuspendEE hangs.
// * IsGCSpecialThread(): we are a GC thread ourselves; the GC wait
// would wait on us.
// * ThreadStore::HoldingThreadStore(pCrashThread): SuspendEE's
// LockThreadStore asserts the holder is unknown, so it would
// assert-fail in checked builds (undefined in release).
// 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.
//
Comment on lines +492 to 496
// The crash reporter is best-effort; on hang the Android watchdog
// kills the process and we keep whatever crash report JSON was flushed
// beforehand.
// Otherwise create a reporter-owned suspension. The result records whether a
// stable suspension is unavailable, inherited, or created by the reporter so
// only the reporter-created suspension is resumed here.
enum class CrashReportSuspensionOwnership
{
Unavailable,
Existing,
Reporter,
};

static
bool
CrashReportSuspensionOwnership
CrashReportSuspendThreads(Thread* pCrashThread)
{
bool crashThreadOwnsSuspension = ThreadStore::HoldingThreadStore(pCrashThread);
if (ThreadSuspend::SysIsSuspended())
{
return crashThreadOwnsSuspension
? CrashReportSuspensionOwnership::Existing
: CrashReportSuspensionOwnership::Unavailable;

@jkotas jkotas Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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?

@mdh1418 mdh1418 Aug 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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?

@jkotas jkotas Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In Server GC, the thread that decides to trigger the GC suspends the runtime. Once the runtime is suspended, it signals to server GC threads to do the GC and waits for them to report back that the GC is done. If you get a crash on server GC thread, it should be safe to assume that the runtime is suspended.

Try to run this program that corrupts GC heap with server GC:

using System.Runtime.CompilerServices;

var o = new Test();
unsafe
{
    fixed (int* p = &o.x)
       *(p-1) = 0x12345678;
}
GC.Collect();
GC.KeepAlive(o);

class Test
{
    public int x;
}

When the GC crashes, you should see the crash on GC thread. The main thread should be waiting at:

coreclr!SVR::gc_heap::wait_for_gc_done+0x52 [D:\a\_work\1\s\src\runtime\src\coreclr\gc\gc.cpp @ 15055] 
coreclr!SVR::GCHeap::GarbageCollectGeneration+0xf8 [D:\a\_work\1\s\src\runtime\src\coreclr\gc\gc.cpp @ 51759] 
coreclr!SVR::GCHeap::GarbageCollect+0xfe [D:\a\_work\1\s\src\runtime\src\coreclr\gc\gc.cpp @ 50890] 
coreclr!GCInterface_Collect+0x66 [D:\a\_work\1\s\src\runtime\src\coreclr\vm\comutilnative.cpp @ 851] 
System_Private_CoreLib!System.GC.Collect+0x64 [/_/src/runtime/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs @ 177] 
repro!Program.<Main>$+0x90 [C:\repro\Program.cs @ 9]

}

if (g_fFatalErrorOccurredOnGCThread
|| GCHeapUtilities::IsGCInProgress()
|| IsGCSpecialThread()
|| ThreadStore::HoldingThreadStore(pCrashThread))
|| crashThreadOwnsSuspension)
{
return false;
return CrashReportSuspensionOwnership::Unavailable;
}

ThreadSuspend::SuspendEE(ThreadSuspend::SUSPEND_OTHER);
return true;
return CrashReportSuspensionOwnership::Reporter;
}

static
Expand Down Expand Up @@ -560,7 +569,7 @@ CrashReportEnumerateThreads(
&crashHresult);
}

bool runtimeSuspended = CrashReportSuspendThreads(pCrashThread);
CrashReportSuspensionOwnership suspensionOwnership = CrashReportSuspendThreads(pCrashThread);

// Emit the crashing thread first so the report keeps the most important
// thread even if later enumeration is incomplete.
Expand All @@ -575,7 +584,7 @@ CrashReportEnumerateThreads(
// Walk the remaining managed threads only when the runtime was
// successfully suspended; otherwise the walker is not guaranteed
// to be at a safe point for them.
if (runtimeSuspended)
if (suspensionOwnership != CrashReportSuspensionOwnership::Unavailable)
{
Thread* pThread = nullptr;
while ((pThread = ThreadStore::GetThreadList(pThread)) != nullptr)
Expand All @@ -591,7 +600,10 @@ CrashReportEnumerateThreads(
CrashReportWalkThread(pThread, frameCallback, ctx);
}

CrashReportResumeThreads();
if (suspensionOwnership == CrashReportSuspensionOwnership::Reporter)
{
CrashReportResumeThreads();
}
}
}

Expand Down
Loading