Skip to content

Revert forwarding activation signal#124877

Open
janvorli wants to merge 1 commit intodotnet:mainfrom
janvorli:revert-forwarding-activation-signal
Open

Revert forwarding activation signal#124877
janvorli wants to merge 1 commit intodotnet:mainfrom
janvorli:revert-forwarding-activation-signal

Conversation

@janvorli
Copy link
Member

Recent change to fix lookup of current thread in the activation handler also added forwarding of the activation signal to the previously registered handler. That part of the change was unrelated to the actual issue the change was fixing. It turned out that it breaks vsdbg that doesn't expect to get the SIGUSR1 used for the activation on macOS.

This change reverts that part of the change to unblock vsdbg.

Recent change to fix lookup of current thread in the activation handler
also added forwarding of the activation signal to the previously
registered handler. That part of the change was unrelated to the actual
issue the change was fixing. It turned out that it breaks vsdbg
that doesn't expect to get the SIGUSR1 used for the activation on macOS.

This change reverts that part of the change to unblock vsdbg.
@janvorli janvorli added this to the 10.0.x milestone Feb 26, 2026
@janvorli janvorli requested review from jkotas and steveisok February 26, 2026 00:38
@janvorli janvorli self-assigned this Feb 26, 2026
Copilot AI review requested due to automatic review settings February 26, 2026 00:38
@janvorli
Copy link
Member Author

/backport to release/10.0

@github-actions
Copy link
Contributor

Started backporting to release/10.0 (link to workflow run)

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Reverts chaining/forwarding of the activation signal handler so that SIGUSR1 used for CoreCLR thread activation on macOS is not forwarded to a previously-registered handler (which breaks vsdbg).

Changes:

  • Stop forwarding the activation signal to g_previous_activation when the runtime accepts/handles the activation.
  • Keep calling the previous handler only in the “not accepted / not for this process” path.

@jkotas
Copy link
Member

jkotas commented Feb 26, 2026

Is there a follow up to do for this?

  • This will re-introduce the potential GC starvation when multiple runtimes are loaded in the process.
  • NativeAOT has the same pattern. Is it not a problem there?

steveisok pushed a commit that referenced this pull request Feb 26, 2026
Backport of #124877 to release/10.0

/cc @janvorli

## Customer Impact

- [ ] Customer reported
- [x] Found internally

A recent change that was backported to .NET 10.0.4
(#124308) was found to cause
problems to vsdbg. That problem is caused by part of the change that was
actually additional to the fix that that change made and that is not
that important. That part of the change started to forward activation
signal to handlers registered before the coreclr. This change just
reverts that part.

## Regression

- [x] Yes
- [ ] No

#124308

## Testing

Local testing

## Risk

Low. It just stops forwarding the signal which was the behavior until
the recent change.

Co-authored-by: Jan Vorlicek <janvorli@microsoft.com>
@janvorli
Copy link
Member Author

/backport to release/11.0-preview2

@github-actions
Copy link
Contributor

Started backporting to release/11.0-preview2 (link to workflow run)

steveisok pushed a commit that referenced this pull request Feb 26, 2026
Backport of #124877 to release/11.0-preview2

/cc @janvorli

## Customer Impact

- [x] Customer reported
- [ ] Found internally

The current CheckActivationSafePoint uses thread local storage to
get the current Thread instance. But this function is called from
async signal handler (the activation signal handler) and it is not
allowed to access TLS variables there because the access can allocate
and if the interrupted code was running in an allocation code, it
could crash.
There was no problem with this since .NET 1.0, but a change in the
recent glibc version has broken this. We've got reports of crashes
in this code e.g on recent Ubuntu 25.04 due to this issue.

## Regression

- [ ] Yes
- [x] No

## Testing
CI tests, local manual directed tests

## Risk
Low, the change was in main for the past two months and we haven't seen
any issues related to it. The newly added code is executed very
frequently.

Co-authored-by: Jan Vorlicek <janvorli@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants