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

Bail ThreadAbort redirection after catch handlers on Unix #72705

Merged
merged 1 commit into from
Jul 26, 2022

Conversation

hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Jul 22, 2022

Fixes #12739 to prevent crashes.

The redirection stub was never implemented, leading to crashes in scenarios like Debugger Function Evaluations. Instead of rethrowing the exception after the catch clause, this allows the ThreadAbortException to flow as any usual exception and hopefully surface through other points that call mechanisms like Thread::HandleThreadAbort. This is a behavior difference between Unix and non-Unix plaforms with some observable side effects, but Thread.Abort is not supported in core. Most commonly the debugger is the major scenario where ThreadAbort gets used, and this prevents the crash.

I've opened #72703 to amend the behavior.

@hoyosjs hoyosjs requested a review from a team July 22, 2022 23:55
@ghost ghost assigned hoyosjs Jul 22, 2022
@hoyosjs hoyosjs added this to the 7.0.0 milestone Jul 22, 2022
@ghost
Copy link

ghost commented Jul 22, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #12739 to prevent crashes.

The redirection stub was never implemented, leading to crashes in scenarios like Debugger Function Evaluations. Instead of rethrowing the exception after the catch clause, this allows the ThreadAbortException to flow as any usual exception and hopefully surface through other points that call mechanisms like Thread::HandleThreadAbort. This is a behavior difference between Unix and non-Unix plaforms with some observable side effects, but Thread.Abort is not supported in core. Most commonly the debugger is the major scenario where ThreadAbort gets used, and this prevents the crash.

I've opened #72703 to amend the behavior.

Author: hoyosjs
Assignees: -
Labels:

area-Diagnostics-coreclr

Milestone: -

@hoyosjs hoyosjs changed the title Bail second pass redirection after catch in thread abort on Unix Bail ThreadAbort redirection after catch handlers on Unix Jul 23, 2022
@hoyosjs hoyosjs marked this pull request as ready for review July 25, 2022 18:34
@hoyosjs
Copy link
Member Author

hoyosjs commented Jul 25, 2022

cc: @AntonLapounov @jkotas I'm merging this today once I get it through debugger testing

@@ -784,7 +784,11 @@ UINT_PTR ExceptionTracker::FinishSecondPass(
{
CopyOSContext(pThread->m_OSContext, pContextRecord);
SetIP(pThread->m_OSContext, (PCODE)uResumePC);
#ifdef TARGET_UNIX
uAbortAddr = NULL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: uAbortAddr is already initialized to the zero value, so this assignment is not needed.

@hoyosjs hoyosjs merged commit d9da50c into main Jul 26, 2022
@hoyosjs hoyosjs deleted the juhoyosa/test-func-fix branch July 26, 2022 09:42
@ghost ghost locked as resolved and limited conversation to collaborators Aug 25, 2022
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.

[macOS/Linux] EXC_BAD_ACCESS when inspecting lazy-loaded objects
4 participants