Skip to content

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

@azure-pipelines
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
@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