Skip to content

Enable runtime-async for SharedFx-only libraries#66200

Open
wtgodbe wants to merge 9 commits intomainfrom
wtgodbe/runtime-async
Open

Enable runtime-async for SharedFx-only libraries#66200
wtgodbe wants to merge 9 commits intomainfrom
wtgodbe/runtime-async

Conversation

@wtgodbe
Copy link
Copy Markdown
Member

@wtgodbe wtgodbe commented Apr 7, 2026

Cribbing off of dotnet/runtime#125406. Enable runtime-async for net11.0+ projects that ship only in the Shared Framework. We can't enable it for projects that ship both in the SharedFx & as packages, because runtime-async is incompatible w/ wasm, so anyone who tried to use such a package in a wasm project would be broken.

Also enables runtime-async for tests.

The wasm exclusion checks both $(TargetOS) != browser and !$(RuntimeIdentifier.StartsWith('browser-')) to correctly exclude projects that target WebAssembly via RuntimeIdentifier=browser-wasm even when $(TargetOS) is empty.

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Description

Enables the runtime-async compiler feature for net11.0+ projects that ship only in the ASP.NET Core Shared Framework (IsAspNetCoreApp=true, IsPackable!=true) and for compatible test/test-asset projects. Projects that are also shipped as NuGet packages are excluded because runtime-async is incompatible with WebAssembly, which would break wasm consumers.

Wasm exclusion is enforced by checking both $(TargetOS) != browser and !$(RuntimeIdentifier.StartsWith('browser-')) to cover the case where $(TargetOS) is empty but the project targets wasm via RuntimeIdentifier=browser-wasm.

Copilot AI review requested due to automatic review settings April 7, 2026 17:57
@github-actions github-actions bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Apr 7, 2026
@wtgodbe wtgodbe requested review from BrennanConroy and agocke April 7, 2026 17:58
Copy link
Copy Markdown
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

Enables the runtime-async feature for .NET 11+ projects that ship only in the shared framework, and extends the same enablement to compatible test projects while attempting to avoid WebAssembly where it’s incompatible.

Changes:

  • Turn on runtime-async for IsAspNetCoreApp=true projects that are not packable and target net11.0+.
  • Turn on runtime-async for IsTestProject / IsTestAssetProject targeting net11.0+.
  • Add a TargetOS != browser condition intended to avoid wasm scenarios.

@wtgodbe wtgodbe added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Apr 7, 2026
<Features>$(Features);runtime-async=on</Features>
</PropertyGroup>

<!-- Also enable runtime async for compatible test projects -->
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.

I assume that the tests would fail if not for this, right? Otherwise, I'd probably want to run in both modes at least temporarily.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think they would, but I'm largely cribbing off of what runtime did: dotnet/runtime#125406

@wtgodbe wtgodbe requested a review from a team as a code owner April 7, 2026 19:20
@wtgodbe
Copy link
Copy Markdown
Member Author

wtgodbe commented Apr 8, 2026

There's now a consistent failure in Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.HttpConnectionManagerTests.CriticalErrorLoggedIfApplicationDoesntComplete

Assert.True() Failure
Expected: True
Actual: False

Looking

@wtgodbe
Copy link
Copy Markdown
Member Author

wtgodbe commented Apr 8, 2026

The test relies on GC to finalize HttpConnection objects — it calls GC.Collect() /
GC.WaitForPendingFinalizers() in a loop and expects the "ApplicationNeverCompleted" log to be triggered within 10 attempts
(line 75).

With runtime-async, the async state machine is different — the runtime keeps the async method's state alive differently than
the traditional compiler-generated state machine. This means the HttpConnection may not be eligible for GC as quickly, causing
the test to need more iterations.

The test already allows up to 30 attempts in the loop (line 69) but asserts that it completes in under 10 (line 75). The
Assert.True(logWaitAttempts < 10) is the assertion that fails — it took 67 seconds total (which is ~30 iterations × ~2s each),
meaning the GC never collected the connection at all.

This is a fundamental behavioral change with runtime-async — the async method's continuation is rooted differently, preventing
the HttpConnection from being finalized.

Verdict: This is a real regression caused by runtime-async. The test depends on GC collecting an HttpConnection whose
application Task never completes — runtime-async changes how the async continuation is rooted, keeping the connection alive.

Fix options:

  1. Opt this test out of runtime-async with [RuntimeAsyncMethodGeneration(false)] — but that only affects the test method, not
    the production code whose async behavior changed
  2. Opt the production code out — the Kestrel connection handling code that's keeping the reference alive
  3. Relax the assertion — but if GC never collects it (30 attempts failed), relaxing won't help
  4. Skip the test when runtime-async is enabled

Since this is a GC-finalization test that's fundamentally incompatible with how runtime-async roots async continuations, the
most pragmatic fix is probably to skip/quarantine it for now and file an issue against the runtime-async feature to ensure
this GC behavior is addressed.

@halter73 @BrennanConroy think we should skip this one for now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants