Skip to content
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

Fix unhandled exception when dependency is missing #99117

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

janvorli
Copy link
Member

ASPNet team has found that the new exception handling doesn't handle unhandled exception caused by a missing dependency assembly correctly. Instead of invoking the unhandled exception machinery in the native code, it actually exited back to the managed exception handler loop with a failure from the the SfiInit.

The reason is that in that specific case, the check if there are still managed frames on the stack that's done by checking if there is any explicit frame doesn't work, since there is a helper method frame.

This change fixes it by moving the unhandled exception check to the end of the SfiInit when we haven't found any managed frame through the stack walker.

It also changes the RaiseFailFastException to RaiseException so that a native host can catch it, which the ASPNet also needs.

I have added a test case for that to the unhandled exception tests.

ASPNet team has found that the new exception handling doesn't handle
unhandled exception caused by a missing dependency assembly correctly.
Instead of invoking the unhandled exception machinery in the native
code, it actually exited back to the managed exception handler loop with
a failure from the the SfiInit.

The reason is that in that specific case, the check if there are still
managed frames on the stack that's done by checking if there is any
explicit frame doesn't work, since there is a helper method frame.

This change fixes it by moving the unhandled exception check to the end
of the SfiInit when we haven't found any managed frame through the stack
walker.
@janvorli janvorli added this to the 9.0.0 milestone Feb 29, 2024
@janvorli janvorli requested a review from jkotas February 29, 2024 15:59
@janvorli janvorli self-assigned this Feb 29, 2024
@janvorli
Copy link
Member Author

/azp run runtime-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BrennanConroy
Copy link
Member

Should you re-enable the new exception handling by default in this PR too?

@janvorli
Copy link
Member Author

Should you re-enable the new exception handling by default in this PR too?

I'll do it in a separate PR

@janvorli janvorli merged commit 8ec2b50 into dotnet:main Feb 29, 2024
186 checks passed
@janvorli janvorli deleted the fix-edge-case-of-unhandled-exception branch February 29, 2024 21:18
@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants