-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix thread abort issue with funceval #118354
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
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 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.
Pull Request Overview
This PR fixes a thread abort issue in function evaluation (funceval) where the process would crash with a failfast error instead of properly timing out. The issue occurred when there were no managed frames between the abortion point and the FuncEvalFrame, such as when invoking static methods whose static constructors hadn't been called yet.
Key changes:
- Unified exception handling by removing conditional compilation around exception dispatch
- Changed from
DispatchManagedException
toRaiseTheExceptionInternalOnly
for all platforms
@janvorli, Thanks a lot for the fix! I confirm the bug is gone :) |
@eterekhin thank you for checking! |
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 thread abort 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 theThreadAbortException
in theThread::HandleThreadAbort
. TheThread::HandleThreadAbort
is always called by native code that has a native catch or (on Windows) ends up calling theProcessCLRException
.I have originally made the change to call the
DispatchManagedException
from theThread::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