Handle Synchronized async versions#129901
Draft
jakobbotsch wants to merge 8 commits into
Draft
Conversation
Non-async Task-returning methods can be Synchronized. We were missing handling for that scenario in the JIT and interpreter.
github-actions
Bot
added
the
area-CodeGen-coreclr
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates CoreCLR’s runtime-async (“async version”) handling for MethodImplOptions.Synchronized methods that return Task / ValueTask without being async, ensuring the monitor is released at the correct point in both the JIT and interpreter paths, and adds a regression test for the scenario.
Changes:
- JIT: avoid async-version “tail await” recognition for synchronized methods and emit an explicit
MON_EXITbefore wrapping the return value in an await. - JIT flowgraph: avoid injecting the usual synchronized-return
MON_EXITblocks for async versions (since they’re now emitted during import). - Interpreter: centralize creation of the synchronized/async return-value temp and adjust async-version return wrapping / tailcall peep behavior.
- Tests: add a new async regression test project covering both
TaskandValueTask.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/async/regression/synchronized-async-version.csproj | Adds a new IL test project for the regression scenario. |
| src/tests/async/regression/synchronized-async-version.cs | Adds xUnit coverage for synchronized non-async Task/ValueTask returns under runtime async. |
| src/coreclr/jit/importer.cpp | Ensures synchronized async versions release the monitor before awaiting/wrapping return values; blocks tail-await recognition for synchronized methods. |
| src/coreclr/jit/flowgraph.cpp | Avoids duplicating synchronized-return monitor-exit insertion for async versions; makes lvaMonAcquired allocation idempotent. |
| src/coreclr/interpreter/compiler.h | Adds declarations/state related to synchronized async-version return handling. |
| src/coreclr/interpreter/compiler.cpp | Adds CreateSynchronizedRetValVar helper and adjusts return/tailcall logic for synchronized async versions. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Open
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Non-async Task-returning methods can be Synchronized. We were missing handling for that async version scenario in the JIT and interpreter.