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

Update some of NativeAOT's uses of Lock to use non-alertable waits #97105

Closed
kouvel opened this issue Jan 17, 2024 · 2 comments · Fixed by #97227 or #98876
Closed

Update some of NativeAOT's uses of Lock to use non-alertable waits #97105

kouvel opened this issue Jan 17, 2024 · 2 comments · Fixed by #97227 or #98876
Assignees
Milestone

Comments

@kouvel
Copy link
Member

kouvel commented Jan 17, 2024

Based on discussion in #94873. Alertable waits can be overridden by SynchronizationContext, are interruptible, and can be reentrant on some threads. Update some low-level uses of Lock such as in class construction to use non-alertable waits.

@kouvel kouvel added this to the 9.0.0 milestone Jan 17, 2024
@kouvel kouvel self-assigned this Jan 17, 2024
@ghost
Copy link

ghost commented Jan 17, 2024

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Based on discussion in #94873. Alertable waits can be overridden by SynchronizationContext, are interruptible, and can be reentrant on some threads. Update some low-level uses of Lock such as in class construction to use non-alertable waits.

Author: kouvel
Assignees: kouvel
Labels:

area-NativeAOT-coreclr

Milestone: 9.0.0

kouvel added a commit to kouvel/runtime that referenced this issue Jan 19, 2024
- Added an internal constructor that enables the lock to use non-alertable waits
- Non-alertable waits are not forwarded to `SynchronizationContext` wait overrides, are non-message-pumping waits, and are not interruptible
- Updated most of the uses of `Lock` in NativeAOT to use non-alertable waits
- Also simplified the fix in dotnet#94873 to avoid having to do the relevant null checks in various places along the wait path, by limiting the scope of the null checks to the initialization phase

Fixes dotnet#97105
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 19, 2024
kouvel added a commit that referenced this issue Jan 24, 2024
…7227)

* Add an internal mode to `Lock` to have it use trivial (non-alertable) waits

- Added an internal constructor that enables the lock to use trivial waits
- Trivial waits are non-alertable waits that are not forwarded to `SynchronizationContext` wait overrides, are non-message-pumping waits, and are not interruptible
- Updated most of the uses of `Lock` in NativeAOT to use trivial waits
- Also simplified the fix in #94873 to avoid having to do the relevant null checks in various places along the wait path, by limiting the scope of the null checks to the initialization phase

Fixes #97105
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 24, 2024
@jkotas
Copy link
Member

jkotas commented Feb 23, 2024

Reverted by #98867

@jkotas jkotas reopened this Feb 23, 2024
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 23, 2024
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 7, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.