Skip to content

Conversation

@janvorli
Copy link
Member

@janvorli janvorli commented Jan 5, 2022

Backport of #59045 to release/6.0

The pthread_kill can fail with ENOTSUP on macOS when the target thread
is a dispatch queue thread. Instead of aborting the process, it is
better to fail to inject the activation and rely on return address
hijacking and other means of syncing with GC.

Customer Impact

Without this change, .NET 6 apps that execute code on dispatch queue threads were reported to abort during GC thread suspension by the Excel team and another 3rd party customer. While Excel team was able to work around the problem by not executing managed code on dispatch queue threads, the other customer was unable to do that due to their heavy usage of these threads.

Testing

coreclr and libraries tests locally. The change has been in main branch for about three months without issues.

Risk

Low, the change only prevents the process from dying immediately when pthread_kill is not supported for the target thread.

Backport of dotnet#59045 to release/6.0

The pthread_kill can fail with ENOTSUP on macOS when the target thread
is a dispatch queue thread. Instead of aborting the process, it is
better to fail to inject the activation and rely on return address
hijacking and other means of syncing with GC.
@janvorli janvorli added Servicing-consider Issue for next servicing release review area-PAL-coreclr labels Jan 5, 2022
@janvorli janvorli added this to the 6.0.x milestone Jan 5, 2022
@janvorli janvorli requested a review from jkotas January 5, 2022 14:13
@janvorli janvorli self-assigned this Jan 5, 2022
@janvorli
Copy link
Member Author

janvorli commented Jan 5, 2022

cc: @christianscheuer

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

Approved. We should take for consideration in 6.0.x


#ifdef __APPLE__
// On Apple, pthread_kill is not allowed to be sent to dispatch queue threads
if (status == ENOTSUP)
Copy link
Member

Choose a reason for hiding this comment

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

Why guard this behind an ifdef? In what condition would we not want to return ERROR_NOT_SUPPORTED?

Copy link
Member Author

Choose a reason for hiding this comment

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

The ENOTSUP error is returned only on apple and in the specific case mentioned. I still prefer abort on an unknown status on other Unix platforms.

@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 6, 2022
@leecow leecow modified the milestones: 6.0.x, 6.0.2 Jan 6, 2022
@jeffschwMSFT jeffschwMSFT merged commit 898f09a into dotnet:release/6.0 Jan 6, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-PAL-coreclr Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants