Fix unhandled exception reporting corner cases #118450
Merged
+26
−34
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are several issues with processing exceptions that are not handled by managed code on Windows: * Sometimes the AppDomain.UnhandledException event is sent multiple times * Exception flowing to foreign native code is reported as unhandled even when it is actually caught by the native code * In rare cases, the unhandled exception stack trace is doubled This change fixes them by not reporting the unhandled exception in the SfiNext on Windows. It just raises the underlying SEH exception there with the thread marked so that the personality routines for the managed frames won't run the managed exception handling code. If the exception is truly unhandled, the `InternalUnhandledExceptionFilter_Worker` will be called by the unhandled exception filter installed for the process and report the exception as unhandled. If that exception ends up being caught by a foreign native code, then nothing will be reported. Close dotnet#115215
There was a problem hiding this comment.
Pull Request Overview
This PR fixes several issues with unhandled exception reporting on Windows by changing how exceptions are processed when they propagate to native code. The main goal is to prevent duplicate unhandled exception events and incorrect reporting of exceptions that are actually caught by foreign native code.
Key changes:
- Renamed thread state flag from
TSNC_UnhandledException2ndPasstoTSNC_SkipManagedPersonalityRoutineto better reflect its purpose - Modified exception handling logic to use
RaiseExceptioninstead of immediately reporting unhandled exceptions inSfiNexton Windows - Streamlined the flow so truly unhandled exceptions are reported by the process-level unhandled exception filter
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/coreclr/vm/threads.h | Renames thread state flag to better reflect its purpose of skipping managed personality routines |
| src/coreclr/vm/exceptionhandling.cpp | Updates exception handling logic to defer unhandled exception reporting and use RaiseException on Windows |
|
The unhandled exceptions test is failing, debug build was passing for me locally, I am investigating what's wrong. |
|
@jkotas I have fixed the ordering of the UnhandledException event and the calls to finally that was causing the test failure in my last commit. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are several issues with processing exceptions that are not handled by managed code on Windows:
This change fixes them by not reporting the unhandled exception in the
SfiNexton Windows. It just raises the underlying SEH exception there with the thread marked so that the personality routines for the managed frames won't run the managed exception handling code. If the exception is truly unhandled, theInternalUnhandledExceptionFilter_Workerwill be called by the unhandled exception filter installed for the process and report the exception as unhandled.If that exception ends up being caught by a foreign native code, then nothing will be reported.
Close #115215