-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Fail fast when FirstChanceExceptionEventArgs allocation fails #130505
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
3cef509
87bfb31
6fc02d2
9976e8c
8ba41c4
5f4fd08
7b8bc2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -130,21 +130,44 @@ internal static void OnUnhandledException(object e) | |||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| [ThreadStatic] | ||||||||||||||||||||
| private static bool t_deliveringFirstChanceNotification; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private static void OnFirstChanceException(Exception e, object? sender) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| if (FirstChanceException is EventHandler<FirstChanceExceptionEventArgs> handlers) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| FirstChanceExceptionEventArgs args = new(e); | ||||||||||||||||||||
| foreach (EventHandler<FirstChanceExceptionEventArgs> handler in Delegate.EnumerateInvocationList(handlers)) | ||||||||||||||||||||
| // Guard against reentrancy. Allocating the event args below or running a | ||||||||||||||||||||
| // handler may itself throw (e.g. OutOfMemoryException in a low-memory | ||||||||||||||||||||
| // situation). That exception would trigger another first-chance | ||||||||||||||||||||
| // notification on this same thread, allocate again, throw again, and | ||||||||||||||||||||
| // recurse until the stack overflows. Skip nested notifications to break | ||||||||||||||||||||
| // the recursion. | ||||||||||||||||||||
| if (t_deliveringFirstChanceNotification) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| try | ||||||||||||||||||||
| { | ||||||||||||||||||||
| handler(sender, args); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| catch | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| t_deliveringFirstChanceNotification = true; | ||||||||||||||||||||
| try | ||||||||||||||||||||
| { | ||||||||||||||||||||
| FirstChanceExceptionEventArgs args = new(e); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| foreach (EventHandler<FirstChanceExceptionEventArgs> handler in Delegate.EnumerateInvocationList(handlers)) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| try | ||||||||||||||||||||
| { | ||||||||||||||||||||
| handler(sender, args); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| catch | ||||||||||||||||||||
| { | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| finally | ||||||||||||||||||||
| { | ||||||||||||||||||||
| t_deliveringFirstChanceNotification = false; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can change https://github.com/dotnet/runtime/blob/main/src/coreclr/System.Private.CoreLib/src/System/AppContext.CoreCLR.cs#L58 to "// The VM does not expect exceptions to propagate out of this callback" (same as
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume you are suggesting a failfast at this point, not just a comment?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am suggesting comment. The VM wraps this in another try/catch that swallows all exceptions - runtime/src/coreclr/vm/excep.cpp Lines 9346 to 9354 in 9904b93
It matches UnhandledException notification. |
||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.