-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix VS debugger issues with funceval and secondary threads #102470
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
Merged
Merged
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
The VS team has recently reported two issues with the new exception handling in Visual Studio debugger. The first issue was that an unhandled exception on a secondary managed thread wasn't showing any stack trace when the exception occured and the debugger has broken in. The other issue was that when an exception occured during a funceval invoked from the immediate window, the debugger would not highlight the source line where the exception occured and it would not display a dialog with the exception details. Both problems were caused by the same underlying problem. In both cases, the "catch handler found" notification was to be sent at a point when the managed stack frames were already gone - in native code in catch / filter. In the funceval case, there was even a check that prevented sending the notification at all when there was no exception info present. The fix is to move the notification to the point where the managed stack frames are still present - when we detect in the EH code that there is no managed frame left and either the DebuggerU2MCatchHandler frame or FuncEvalFrame is the explicit frame we've encountered as the next one to process. The FuncEvalFrame case is a bit more involved, as we always push ProtectValueClassFrame after the FuncEvalFrame, so we need to skip that one in the check. The debugger actually needs to get a pointer to the FuncEvalFrame in the notification to do the right thing. Close dotnet#102178 and dotnet#101729
jkotas
approved these changes
May 20, 2024
tommcdon
approved these changes
May 20, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
The ProtectValueClassFrame can also occur without FuncEvalFrame in the reflection invocation.
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
The test failures are known and unrelated to this change. |
Ruihan-Yin
pushed a commit
to Ruihan-Yin/runtime
that referenced
this pull request
May 30, 2024
…2470) * Fix VS debugger issues with funceval and secondary threads The VS team has recently reported two issues with the new exception handling in Visual Studio debugger. The first issue was that an unhandled exception on a secondary managed thread wasn't showing any stack trace when the exception occured and the debugger has broken in. The other issue was that when an exception occured during a funceval invoked from the immediate window, the debugger would not highlight the source line where the exception occured and it would not display a dialog with the exception details. Both problems were caused by the same underlying problem. In both cases, the "catch handler found" notification was to be sent at a point when the managed stack frames were already gone - in native code in catch / filter. In the funceval case, there was even a check that prevented sending the notification at all when there was no exception info present. The fix is to move the notification to the point where the managed stack frames are still present - when we detect in the EH code that there is no managed frame left and either the DebuggerU2MCatchHandler frame or FuncEvalFrame is the explicit frame we've encountered as the next one to process. The FuncEvalFrame case is a bit more involved, as we always push ProtectValueClassFrame after the FuncEvalFrame, so we need to skip that one in the check. The debugger actually needs to get a pointer to the FuncEvalFrame in the notification to do the right thing. Close dotnet#102178 and dotnet#101729 * Fix too strong assert The ProtectValueClassFrame can also occur without FuncEvalFrame in the reflection invocation.
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.
The VS team has recently reported two issues with the new exception handling in Visual Studio debugger.
The first issue was that an unhandled exception on a secondary managed thread wasn't showing any stack trace when the exception occured and the debugger has broken in.
The other issue was that when an exception occured during a funceval invoked from the immediate window, the debugger would not highlight the source line where the exception occured and it would not display a dialog with the exception details.
Both problems were caused by the same underlying problem. In both cases, the "catch handler found" notification was to be sent at a point when the managed stack frames were already gone - in native code in catch / filter. In the funceval case, there was even a check that prevented sending the notification at all when there was no exception info present.
The fix is to move the notification to the point where the managed stack frames are still present - when we detect in the EH code that there is no managed frame left and either the DebuggerU2MCatchHandler frame or FuncEvalFrame is the explicit frame we've encountered as the next one to process. The FuncEvalFrame case is a bit more involved, as we always push ProtectValueClassFrame after the FuncEvalFrame, so we need to skip that one in the check. The debugger actually needs to get a pointer to the FuncEvalFrame in the notification to do the right thing.
Close #102178 and #101729