-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Deliver exceptions to FuncEvalFrame and DebuggerU2MCatchHandlerFrame when they are higher than the top managed frame in stack #118015
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
Conversation
…when they are higher than the top managed frame in stack
@eterekhin thank you for looking into this issue and creating a fix! I think the fix should be made differently though. For example, the |
@janvorli, Thank you! That's great you are going to fix this case! |
@janvorli, Hello! I've found a stable repro, Win x64, .NET 9
After 6 seconds waiting I see (please see the screenshot)
![]() |
@eterekhin thank you! I'll use it to verify my changes. |
There is a problem with threadabort in funceval in case there is no managed frame on the stack between the abortion point and the `FuncEvalFrame`. That can happen e.g. when invoking a static method via funceval for a type with static constructor that was not invoked yet and takes a long time to complete. The problem is caused by the fact that when EH is called to propagate the ThreadAbortException, it starts at the first managed frame and so it skips the try/catch in the funceval native code. This change fixes it by using `RaiseTheExceptionInternalOnly` to raise the `ThreadAbortException` in the `Thread::HandleThreadAbort`. The `Thread::HandleThreadAbort` is always called by native code that has a native catch or (on Windows) ends up calling the `ProcessCLRException`. I have originally made the change to call the `DispatchManagedException` from the `Thread::HandleThreadAbort`, but this issue shows it is problematic. I have verified that the repro provided by @eterekhin in the issue report no longer causes the process to crash with failfast, but reports the funceval as timed out as expected. Close dotnet#118015
There is a problem with threadabort in funceval in case there is no managed frame on the stack between the abortion point and the `FuncEvalFrame`. That can happen e.g. when invoking a static method via funceval for a type with static constructor that was not invoked yet and takes a long time to complete. The problem is caused by the fact that when EH is called to propagate the ThreadAbortException, it starts at the first managed frame and so it skips the try/catch in the funceval native code. This change fixes it by using `RaiseTheExceptionInternalOnly` to raise the `ThreadAbortException` in the `Thread::HandleThreadAbort`. The `Thread::HandleThreadAbort` is always called by native code that has a native catch or (on Windows) ends up calling the `ProcessCLRException`. I have originally made the change to call the `DispatchManagedException` from the `Thread::HandleThreadAbort`, but this issue shows it is problematic. I have verified that the repro provided by @eterekhin in the issue report no longer causes the process to crash with failfast, but reports the funceval as timed out as expected. Close #118015
There is a problem with threadabort in funceval in case there is no managed frame on the stack between the abortion point and the `FuncEvalFrame`. That can happen e.g. when invoking a static method via funceval for a type with static constructor that was not invoked yet and takes a long time to complete. The problem is caused by the fact that when EH is called to propagate the ThreadAbortException, it starts at the first managed frame and so it skips the try/catch in the funceval native code. This change fixes it by using `RaiseTheExceptionInternalOnly` to raise the `ThreadAbortException` in the `Thread::HandleThreadAbort`. The `Thread::HandleThreadAbort` is always called by native code that has a native catch or (on Windows) ends up calling the `ProcessCLRException`. I have originally made the change to call the `DispatchManagedException` from the `Thread::HandleThreadAbort`, but this issue shows it is problematic. I have verified that the repro provided by @eterekhin in the issue report no longer causes the process to crash with failfast, but reports the funceval as timed out as expected. Close dotnet#118015
Hey, folks!
This PR fixes a crash because of eval abort, when abort called before the method actually runs (no managed frames related to the eval are on the stack at this moment, please see the stack trace attached below).
Debugger calls
ICorDebugEval::CallFunction
,ICorDebugProcess::Continue
andICorDebugEval::Abort
sequentially.When a being evaluated method already runs at the moment when
Abort
is performed,ThreadAbortException
is propagated to the exception handler added inGCProtectArgsAndDoNormalFuncEval
, but when it doesn't, we unwound the exception to a frame where debugger was stopped, missing the exception handler, that leads to a process crash during first exception pass hereIt reproduces in .NET 9 installation on all OS and in main, .NET 8 works fine for me
This problem araises when runtime executes the class constructor before the call, so it takes some time to get to the call itself
In this PR I check for this situation in
SfiInit
and setpfIsExceptionIntercepted
flag, by that we skipSfiNext
calls and go straight toCallCatchFunclet
. I also added check for DebuggerU2MCatchHandlerFrame because I guess we may hit this issue for it as well@janvorli, may I ask you to review it, please? I have probably missed some pieces :) Also not sure the tests will be green