Implement ExceptionHandling.SetFatalErrorHandler

[AaronRobinsonMSFT]

Implements the ExceptionHandling.SetFatalErrorHandler API (#101560) for both NativeAOT and CoreCLR. Mono throws PlatformNotSupportedException.

Changes

Managed API (ExceptionHandling.cs)

• Mono throws PNSE

NativeAOT (RuntimeExceptionHelpers.cs)

• Supported.

CoreCLR (eepolicy.cpp)

• Supported.

Public native header (src/native/public/FatalErrorHandling.h)

• Defines FatalErrorHandlerResult enum, FatalErrorInfo struct, callback typedefs

Tests (src/tests/baseservices/exceptions/FatalErrorHandler/)

• Subprocess-based tests validating: SkipHandler, RunHandler, LogHandler, SetNull, SetTwice
• Works on both CoreCLR and NativeAOT

Fixes #101560

[Copilot]

Pull request overview

This PR introduces the public System.Runtime.ExceptionServices.ExceptionHandling.SetFatalErrorHandler API and wires it up so CoreCLR and NativeAOT invoke a user-provided unmanaged callback during fatal-error paths, with a mechanism to retrieve the fatal-error log text.

Changes:

• Adds ExceptionHandling.SetFatalErrorHandler(delegate* unmanaged<int, void*, int>) to the public surface and implements registration in System.Private.CoreLib.
• Implements fatal-error handler invocation + crash-log capture in both CoreCLR (VM) and NativeAOT fail-fast paths.
• Adds a new native public header (FatalErrorHandling.h) and a new subprocess-based test covering handler behaviors.

Show a summary per file

File	Description
src/tests/baseservices/exceptions/FatalErrorHandler/FatalErrorHandlerTest.csproj	Adds new standalone test project for fatal error handler scenarios.
src/tests/baseservices/exceptions/FatalErrorHandler/FatalErrorHandlerTest.cs	Subprocess-based validation of handler invocation, skip/run default behavior, and log retrieval.
src/native/public/FatalErrorHandling.h	Defines native ABI structs/enums/callback types for fatal error handling and log retrieval.
src/libraries/System.Runtime/ref/System.Runtime.cs	Adds the new public ref-assembly API for SetFatalErrorHandler.
src/libraries/System.Private.CoreLib/src/System/Runtime/ExceptionServices/ExceptionHandling.cs	Implements handler registration and stores function pointer for runtimes to read.
src/libraries/System.Private.CoreLib/src/Resources/Strings.resx	Adds resource string for duplicate fatal handler registration.
src/coreclr/vm/util.hpp	Declares crash-log capture helpers used by fatal-error handler plumbing.
src/coreclr/vm/util.cpp	Implements stderr “tee” into a fixed crash-log buffer.
src/coreclr/vm/eepolicy.cpp	Invokes the fatal handler after logging fatal errors / stack overflow and provides log callback.
src/coreclr/vm/corelib.h	Adds CoreLibBinder field binding for ExceptionHandling.s_fatalErrorHandler.
src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeExceptionHelpers.cs	Captures crash output into a buffer and invokes the fatal handler before default crash processing.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 5

[Copilot]

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 6

[Copilot]

Copilot's findings

• Files reviewed: 17/17 changed files
• Comments generated: 4

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 4

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 4

[janvorli]

We can possibly have multiple threads failing at the same time. It seems that we could end up trying to write past the end of the fragments array.

[AaronRobinsonMSFT]

Would you suggest I try and allocate or attempt to follow the mechanism in CoreCLR that avoids the buffer, but uses TLS?

[janvorli]

My point was mostly about preventing possible crash due to out of bounds access to the s_crashLogFragments due to the way the s_crashLogFragmentCount is incremented. So the s_crashLogFragmentCount >= MaxCrashLogFragments may pass and then another thread would increment the s_crashLogFragmentCount before the current thread adds the entry.

[Copilot]

Copilot's findings

Comments suppressed due to low confidence (2)

src/libraries/System.Private.CoreLib/src/System/Runtime/ExceptionServices/ExceptionHandling.cs:82

• On Mono, the method throws PlatformNotSupportedException before validating handler for null. The approved API comment on #101560 documents ArgumentNullException for a null handler; argument validation should generally happen before platform checks so callers get consistent exception behavior.

        public static unsafe void SetFatalErrorHandler(delegate* unmanaged<int, void*, int> handler)
        {
#if MONO
            throw new PlatformNotSupportedException();
#else
            ArgumentNullException.ThrowIfNull((void*)handler, nameof(handler));

            if (!TrySetFatalErrorHandler((IntPtr)handler))
            {
                throw new InvalidOperationException(SR.InvalidOperation_CannotRegisterSecondFatalErrorHandler);
            }
#endif

src/native/public/FatalErrorHandling.h:7

• The PR description / linked issue text describes a FatalErrorInfo struct-based contract, but this header actually defines a property-getter callback model (no FatalErrorInfo struct). Please either update the PR description/docs to match the implemented contract, or adjust the header/managed docs if the struct-based design is still the intended API.

// This header defines the native types used by the
// ExceptionHandling.SetFatalErrorHandler API. A native fatal error handler
// receives an HRESULT and a property-getter callback through which it can
// request additional crash information on demand.

• Files reviewed: 29/29 changed files
• Comments generated: 0 new

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "3b168438f67eb0478f255d4a21a55bb4b41186fe",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "c9265d78e03e96c00b7ec062f2dbf2465bbb86d6",
  "last_reviewed_commit": "3b168438f67eb0478f255d4a21a55bb4b41186fe",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "c9265d78e03e96c00b7ec062f2dbf2465bbb86d6",
  "last_recorded_worker_run_id": "29687173886",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "3b168438f67eb0478f255d4a21a55bb4b41186fe",
      "review_id": 4730782393
    }
  ]
}

[github-actions]

Holistic Review

Motivation: Strong and well-established. This implements the api-approved ExceptionHandling.SetFatalErrorHandler (issue #101560), a long-requested capability (crash reporters like breakpad/Sentry behind the .NET runtime) with clear demand from the linked issue's discussion. The problem is real and the direction was reviewed by API review.

Approach: Reasonable and consistent with the codebase. The public managed surface exactly matches the approved shape (namespace, SetFatalErrorHandler(delegate* unmanaged<int, void*, int>), [CLSCompliant(false)], ANE/IOE/PNSE exceptions). The shared native contract lives in a new public header (FatalErrorHandling.h), registration uses a lock-free InterlockedCompareExchange single-registration model, and crash output is refactored behind a CrashInfoWriter abstraction so the same text can go to stderr or the user's log callback. CoreCLR hooks HandleFatalError/HandleFatalStackOverflow; NativeAOT hooks the classlib FailFast plus the Unix signal handlers and a new Windows last-chance unhandled-exception filter. The GetRuntimeException classlib export signature change (adding faultingIP) is threaded consistently through all call sites and Test.CoreLib.

Summary: ⚠️ Needs Human Review. The API shape is approved and correctly implemented, and the change is thorough with a genuinely comprehensive subprocess-based test matrix (skip/run/log/native/nested/set-null/set-twice across CoreCLR and NativeAOT). My main reservations are around the fatal-path robustness of invoking arbitrary user code before the runtime's crashing-thread latch and the documented re-entrancy/concurrency contract — see the inline comments. These are behavioral/contract questions best judged by the exceptions/EH maintainers rather than blocking defects I can confirm. As a new public runtime API with platform-specific EH surgery, this warrants careful human review, and I could not run builds or tests in this environment.

---

Detailed Findings

✅ API Approval — Public surface matches the approved proposal

The ref/System.Runtime.cs addition and the src implementation match the api-approved comment on issue #101560 exactly: System.Runtime.ExceptionServices.ExceptionHandling.SetFatalErrorHandler(delegate* unmanaged<int, void*, int> handler), [CLSCompliant(false)], with ArgumentNullException/InvalidOperationException/PlatformNotSupportedException. No extra or missing public managed surface was found. The new src/native/public/FatalErrorHandling.h is a native (non-managed-ref) contract and is appropriately not part of the managed ref assembly.

⚠️ Fatal-path robustness / re-entrancy — see inline on eepolicy.cpp (InvokeFatalErrorHandler)

User code now runs on the crashing thread before the s_pCrashingThreadID latch (which moved into LogInfoForFatalError, only on the RunDefaultHandler path). This exposes concurrent and re-entrant handler invocation that the FatalErrorHandling.h contract does not document. Detailed inline comment provided; the same invocation pattern exists at the NativeAOT sites.

💡 Skip-path control flow — see inline on eepolicy.cpp (skip SafeExitProcess)

Minor defense-in-depth / consistency: the SkipDefaultHandler SafeExitProcess has no trailing UNREACHABLE() unlike the stack-overflow site, so a hypothetical return would fall through and emit the suppressed crash log.

✅ Cross-platform consistency — Windows filter vs. Unix signal handlers

The genuinely-unmanaged fatal path is handled symmetrically: Unix SIGSEGVHandler/SIGFPEHandler gate on ShouldSkipDefaultHandlingForNativeException and restore the previous disposition on skip; Windows adds a chained last-chance SetUnhandledExceptionFilter that preserves any previously-installed filter. IsFatalHardwareExceptionForFatalErrorHandler mirrors the same fault-code set on both CoreCLR/Windows and NativeAOT/Windows, and stack overflow is consistently and deliberately excluded (documented) because too little stack remains. One observation for the author/human reviewer: NativeAOT/Unix installs SIGSEGV and SIGFPE handlers but not SIGILL, while the fault-code lists include illegal/privileged-instruction codes — worth confirming that illegal-instruction native faults are intentionally out of scope on Unix.

✅ Test quality — comprehensive subprocess matrix

The test spawns child processes per scenario and asserts on stderr markers and exit codes: SkipHandler (crash log suppressed), RunHandler (default log emitted exactly once — with an explicit regression guard against the double-emission "Fatal error while logging another fatal error." path), LogHandler (pfnGetFatalErrorLog round-trip), native/native-code/nested-hardware-fault address+platform-record propagation, and the SetNull/SetTwice argument-validation paths. Platform/runtime applicability is gated correctly (e.g. native-code path skipped on CoreCLR/Unix, nested-fault path NativeAOT-only). RequiresProcessIsolation and the Mono CLRTestTargetUnsupported gate are set appropriately. This is a good, behavior-focused test suite.

💡 Minor naming/consistency observations (non-blocking)

• The XML doc on SetFatalErrorHandler and the header comment reference pfnGetFatalErrorLog, but the managed doc refers to FatalErrorPropertyGetter; the naming across the doc comment, header, and code is mostly consistent but a human may want to confirm the doc-comment terminology (RunDefaultHandler/SkipDefaultHandler values, property getter) reads cleanly for the public API.
• The NativeAOT FatalErrorProperty enum is duplicated in managed code with a "must be kept in sync" comment against the native header — acceptable, but a drift risk worth a human's awareness.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 412.1 AIC · ⌖ 12 AIC · ⊞ 10K

[github-actions]

⚠️ Handler re-entrancy / concurrency exposure. InvokeFatalErrorHandler reads s_fatalErrorHandler and calls the user callback, but nothing here (or in the caller) guards against re-entry or concurrent entry:

• If the user handler itself faults or calls FailFast, HandleFatalError/HandleFatalStackOverflow runs again, s_fatalErrorHandler is still non-null, and the handler is invoked recursively with no bound — potentially unbounded recursion before the process finally dies.
• The s_pCrashingThreadID single-crashing-thread guard now lives in LogInfoForFatalError, which only runs on the RunDefaultHandler path. Two threads crashing simultaneously will both enter InvokeFatalErrorHandler and call the user callback concurrently, before that guard is reached.

The public contract in FatalErrorHandling.h doesn't document that the callback can be invoked re-entrantly or concurrently. Consider either serializing/guarding handler invocation (e.g. reuse a crashing-thread latch around the invoke) or explicitly documenting the re-entrant/concurrent contract. The same concern applies to the NativeAOT invocation sites in RuntimeExceptionHelpers.cs (FailFast and InvokeFatalErrorHandlerForNativeException).

[github-actions]

💡 On the SkipDefaultHandler path, SafeExitProcess(exitCode, SCA_ExitProcessWhenShutdownComplete) is expected to terminate the process, but unlike the stack-overflow site (line ~1010) there is no UNREACHABLE() after it. If SafeExitProcess ever returned here (it takes SCA_ExitProcessWhenShutdownComplete, which calls ExitProcess, so normally it won't), control would fall through into the RunDefaultHandler block below and emit the crash log the handler explicitly asked to suppress. Consider adding UNREACHABLE(); after the skip-path SafeExitProcess for consistency and defense-in-depth, mirroring the SO path.