Skip to content

JIT: Exclude byref-like structs from async continuation capture - #136085

Merged
EgorBo merged 1 commit into
dotnet:mainfrom
EgorBo:fix-runtime-async-ref-struct-capture
Oct 19, 2026
Merged

JIT: Exclude byref-like structs from async continuation capture#136085
EgorBo merged 1 commit into
dotnet:mainfrom
EgorBo:fix-runtime-async-ref-struct-capture

Conversation

@EgorBo

@EgorBo EgorBo commented Oct 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #136000.

Exclude byref-like structs from async continuation capture even when they contain no byrefs. Capturing these structs can emit heap stores without write barriers because their layouts are considered stack-only, causing GC corruption when continuations are reused.

The runtime-async spec states:

Byref-like structs will also not be hoisted across suspension points and will have their default value after a suspension point.

I assume we might allow them actually (that will require a few changes in JIT to stop assuming that "ref struct" means no WB needed in the context of async-v2), but since the spec explicitly says so...

@EgorBo
JIT: Exclude byref-like structs from async continuation capture
1e36cc4 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a18055aa-e854-4590-b3ee-c17c341bde34
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 19, 2026
@azure-pipelines

azure-pipelines Bot commented Oct 19, 2026

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

dotnet-policy-service Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

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

@EgorBo

EgorBo commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

Since @jakobbotsch is currently OOO, can @VSadov or @davidwrighton take a look if that is the right fix?

@build-analysis build-analysis Bot mentioned this pull request Oct 19, 2026
Open
@VSadov

VSadov commented Oct 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

I assume we might allow them actually (that will require a few changes in JIT to stop assuming that "ref struct" means no WB needed in the context of async-v2), but since the spec explicitly says so..

It was considered, but there is not much advantage in making byref structs with no byref fields (recursively) an exception from the rules - only extra complexity in the spec.

Besides, the cases where byref structs do not have byref fields is often a pattern when the struct wants to be “pinned” to the thread for correctness - like Lock.Scope, ObjectHandleOnStack,...
Allowing them to be captured will break their thread affinity expectations.

@EgorBo

EgorBo commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

It was considered, but there is no advantage in making byref structs with no byref fields (recursively) an exception from the rules - only extra complexity in the spec.

It was once mentioned that we could build ArrayPool that returns a ref struct holding an object and it could survive the async/await boundary with async-v2, but not sure if that is still considered (probably replaced with broader ownership feature).

Well, then this PR is correct, right? Because it rejects such cases.

jkotas
jkotas reviewed Oct 19, 2026
View reviewed changes
Comment thread src/coreclr/jit/asyncanalysis.cpp
VSadov
VSadov approved these changes Oct 19, 2026
View reviewed changes

@VSadov VSadov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@EgorBo

EgorBo commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

/ba-g failure is #136096

@EgorBo
EgorBo enabled auto-merge (squash) October 19, 2026 23:05
@EgorBo
EgorBo merged commit 9a88a84 into dotnet:main Oct 19, 2026
144 of 147 checks passed
@EgorBo EgorBo mentioned this pull request Oct 19, 2026
Open
@dotnet-maestro dotnet-maestro Bot mentioned this pull request Oct 20, 2026
Merged
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Oct 20, 2026
@EgorBo EgorBo mentioned this pull request Oct 20, 2026
Open
2 of 4 tasks
This was referenced Oct 20, 2026
Closed
Merged
radekdoulik added a commit that referenced this pull request Oct 20, 2026
@radekdoulik
[clr-interp] Preserve byref-like locals when an await does not suspend (
6f6de27 
#136172)

<!-- -->

## Summary

Move the existing no-continuation fast path ahead of local zeroing in
the shared interpreter continuation handler.

For a synchronously completed await, the method does not suspend and
must preserve its initialized locals. The existing order clears those
locals before taking the no-continuation fast path.

For actual suspension, retain the existing zeroing before handling the
continuation. Both the ordinary and generic continuation opcodes share
this handler.

## Regression coverage

Expand the existing `Runtime_136000.TestEntryPoint` into a four-case
conditional theory: generic/nongeneric containing type crossed with
ref-struct/byref local. Every case checks preservation across
`Task.CompletedTask`, an incomplete result for a pending task, and
defaulting after actual suspension.

Use a no-inline ref identity helper for the ordinary byref, analogous to
the original payload's opaque initialization. This keeps a byref local
represented so the test exercises continuation-local clearing rather
than recomputation of a local's address. Checked interpreter dumps
confirm both ordinary and generic continuation handlers are exercised.
Existing feature conditions are retained; no new files or skips are
added.

A direct-address prototype exposed a separate native-JIT
address-rematerialization issue after suspension. The opaque initializer
isolates the interpreter-local clearing path; this PR does not change
JIT code or disable the byref cases.

The test was added by
[#136085](#136085), which fixes a
different native-JIT continuation-capture problem. The interpreter's
clearing-before-check behavior predates that test.

## Validation

Built actual CoreCLR/browser-Wasm Checked runtimes and Release managed
libraries from `d260b5c598ab285046e217c012f6d7126947b06a`, then rebuilt
the corrected native Wasm runtime and refreshed Core_Root. Also built
the same-source macOS/arm64 Checked runtime for a native-JIT control.

| Validation | Result |
|---|---|
| Unchanged original regression, fresh unfixed Wasm runtime | 1 expected
first-assertion failure |
| Final expanded regression, retained unfixed Wasm runtime | 16/16
expected first-assertion failures |
| Byte-identical original regression, fixed Wasm runtime | 4/4 passed |
| Final expanded regression, fixed Wasm runtime | 16/16 passed |
| Related async coverage through the generated browser wrapper | 36/36
passed |
| Final expanded regression, same-source native ARM64 JIT | 4/4 passed |

The final positive runs contain **60 passing selected test-case
executions, zero selected failures, and zero selected skips**.
Filtered-out tests are not counted as executed coverage.

Wasm runs used Node.js v26.11.0 and the four `(DOTNET_ReadyToRun,
DOTNET_TieredCompilation)` settings `(0,1)`, `(0,0)`, `(1,1)`, and
`(1,0)`. The broader coverage comprises nine selected async cases per
setting: shared generics, byref/out parameters, array updates, void
results, generic virtual awaits and exception propagation, and emitted
runtime-async methods.

The native control disables R2R and interpretation. Eight FullOpts ARM64
emitted-method assembly listings verify actual native JIT compilation.
The final native and Wasm regression DLLs are byte-identical:
`74ae40379b4da41bfbc31c56e87ea515398a13a4053238b599ab4355f62fecf3`.

Commands ran from the repository root with
`DOTNET_WASM_TOOL_CACHE_DIR="$PWD/.dotnet/wasm-tools"`.

```bash
# Clean browser/Wasm baseline: passed on the single identical retry.
./build.sh clr+libs+host -os browser -c Release -rc Checked

# Original regression, related async runner, and initial test layout: passed.
src/tests/build.sh -os browser -arch wasm Checked -skipnative -priority1 \
  -test:JIT/Regression_ro_2/Regression_ro_2.csproj \
  -test:async/async.csproj /p:TasksConfiguration=Release

# Corrected native Wasm runtime, including interpexec.cpp and host relinking: passed.
./build.sh clr -os browser -c Release -rc Checked

# Refresh the browser test layout: passed.
src/tests/build.sh -os browser -arch wasm Checked -generatelayoutonly \
  /p:TasksConfiguration=Release

# Same-source native macOS/arm64 runtime: passed.
./build.sh clr+libs+host -os osx -arch arm64 -c Release -rc Checked

# Initial native regression and Core_Root layout: passed.
src/tests/build.sh -os osx -arch arm64 Checked -skipnative -priority1 \
  -test:JIT/Regression_ro_2/Regression_ro_2.csproj \
  /p:TasksConfiguration=Release

# Final regression build, native: passed.
src/tests/build.sh -os osx -arch arm64 Checked -skipnative \
  -skipgeneratelayout -priority1 \
  -test:JIT/Regression_ro_2/Regression_ro_2.csproj \
  /p:TasksConfiguration=Release

# Final regression build, browser/Wasm: passed.
src/tests/build.sh -os browser -arch wasm Checked -skipnative \
  -skipgeneratelayout -priority1 \
  -test:JIT/Regression_ro_2/Regression_ro_2.csproj \
  /p:TasksConfiguration=Release
```

The initial clean baseline hit an MSB3883 compiler-lock exception before
any source edits. The one identical retry passed without a source or
build-setting workaround; all listed successful build commands completed
with zero warnings and errors.

Representative final regression invocations, with `root` set to the
repository root and each run executed in a separate output directory:

```bash
# Browser/Wasm: 4 selected passes; repeat for each switch combination above.
export CORE_ROOT="$root/artifacts/tests/coreclr/browser.wasm.Checked/Tests/Core_Root"
DOTNET_ReadyToRun=0 DOTNET_TieredCompilation=0 \
  node --experimental-wasm-exnref --stack-size=8192 \
  "$root/artifacts/bin/coreclr/browser.wasm.Checked/corerun.js" -c "$CORE_ROOT" \
  "$root/artifacts/tests/coreclr/browser.wasm.Checked/JIT/Regression_ro_2/Regression_ro_2/Regression_ro_2.dll" \
  Runtime_136000

# Native ARM64 JIT: 4 selected passes, with native assembly listings.
export CORE_ROOT="$root/artifacts/tests/coreclr/osx.arm64.Checked/Tests/Core_Root"
DOTNET_ReadyToRun=0 DOTNET_TieredCompilation=0 DOTNET_InterpMode=0 \
  DOTNET_Interpreter='' DOTNET_JitDisasm='*:Test' \
  "$CORE_ROOT/corerun" \
  "$root/artifacts/tests/coreclr/osx.arm64.Checked/JIT/Regression_ro_2/Regression_ro_2/Regression_ro_2.dll" \
  Runtime_136000
```

The merged runner can return process exit 100 even for failed tests.
Results above come from the selected xUnit XML entries and exact-count
checks, not process exit status alone. Broader browser async tests were
run through their generated `async.sh` wrapper for host-side subprocess
orchestration.

## Scope

The product correction only changes ordering in the shared interpreter
handler. Real-suspension zeroing and continuation capture remain
unchanged. No native JIT source, Reflection.Emit support, R2R folding,
pipeline coverage, or reporting automation is changed.

Validation uses the actual browser/Wasm target through Node, not a
Chrome tab or the exact CI binaries. The ReadyToRun settings above are
environment controls, **not verified R2R-compiled callers or composite
CI payloads**. No full async suite, Mono, or other native architectures
are claimed.

Resolves #136171

> [!NOTE]
> This pull request description was generated with GitHub Copilot.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b691535f-9dc8-49e3-95fe-2e83af8ef957
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkotas jkotas jkotas approved these changes
@VSadov VSadov VSadov approved these changes
@davidwrighton davidwrighton Awaiting requested review from davidwrighton

Assignees

@EgorBo EgorBo

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI runtime-async

Projects

None yet

Milestone

12.0-preview1

Development

Successfully merging this pull request may close these issues.

RuntimeAsync: a byref-like struct local holding object references is stored into the continuation without a GC write barrier

3 participants

@EgorBo @VSadov @jkotas