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

Fix class construction recursion issue in Lock on NativeAOT #94241

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

kouvel
Copy link
Member

@kouvel kouvel commented Oct 31, 2023

  • The Monitor type was being constructed due to the use of Monitor.DebugBlockingScope, added that to the initialization phase so that the relevant type is initialized before it's used on the wait path
  • A better alternative may be to move DebugBlockingScope to be under Lock, it would probably make more sense for Monitor to refer to Lock as it already does, than the inverse. Based on the comments the thread-static field is apparently bound-to in debugging scenarios currently. Something to look into, for now this should unblock the CI.
  • Fixes NativeAOT tests crashing with infinite recursion in Lock #94227

- The `Monitor` type was being constructed due to the use of `Monitor.DebugBlockingScope`, added that to the initialization phase
- If necessary, an alternative may be to move `DebugBlockingScope` to be under `Lock`. Based on the comments the thread-static field is apparently bound-to in debugging scenarios.
- Fixes dotnet#94227
@kouvel kouvel added this to the 9.0.0 milestone Oct 31, 2023
@kouvel kouvel requested a review from VSadov October 31, 2023 21:02
@ghost
Copy link

ghost commented Oct 31, 2023

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

Issue Details
  • The Monitor type was being constructed due to the use of Monitor.DebugBlockingScope, added that to the initialization phase so that the relevant type is initialized before it's used on the wait path
  • A better alternative may be to move DebugBlockingScope to be under Lock, it would probably make more sense for Monitor to refer to Lock as it already does, than the inverse. Based on the comments the thread-static field is apparently bound-to in debugging scenarios currently. Something to look into, for now this should unblock the CI.
  • Fixes NativeAOT tests crashing with infinite recursion in Lock #94227
Author: kouvel
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: 9.0.0

@jkotas
Copy link
Member

jkotas commented Oct 31, 2023

Based on the comments the thread-static field is apparently bound-to in debugging scenarios currently.

These comments are left-overs from .NET Native for UWP. No debugger uses this field today.

@VSadov
Copy link
Member

VSadov commented Nov 1, 2023

These comments are left-overs from .NET Native for UWP. No debugger uses this field today.

Although it did not cause problems before, if it is useless, perhaps removing DebugBlockingScope entirely could be a better "fix"?

@kouvel
Copy link
Member Author

kouvel commented Nov 1, 2023

These comments are left-overs from .NET Native for UWP. No debugger uses this field today.

Ah ok, thanks!

perhaps removing DebugBlockingScope entirely could be a better "fix"?

Sounds good to me, updated. Something like that could always be added as necessary.

@jkotas
Copy link
Member

jkotas commented Nov 1, 2023

Sounds good to me, updated. Something like that could always be added as necessary.

Yes, we should look into whether there is any diagnostic support required for the new Lock type, across all runtimes.

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.

NativeAOT tests crashing with infinite recursion in Lock
3 participants