Skip to content

Runtime async: keep stack traces and avoid allocation when rewriting suspending handlers - #20870

Open
majocha wants to merge 4 commits into
dotnet:mainfrom
majocha:runtime-async-rethrow-dispatch-info
Open

majocha wants to merge 4 commits into
dotnet:mainfrom
majocha:runtime-async-rethrow-dispatch-info

Conversation

@majocha

@majocha majocha commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

A try/with handler or try/finally compensation that suspends is moved out of its exception-handling region. The rewrite recorded the body's outcome in a Choice<'T, exn>, allocated on every run including success, and rethrew a pending exception with a plain throw, which reset its stack trace. This affected a suspending finally (including use of an IAsyncDisposable), reraise () after a suspension, and an unmatched handler pattern.

Fix:

  • Record the body's result and the pending exception in two locals; the exception stays null when the body succeeds, so nothing is allocated.
  • Rethrow with ExceptionDispatchInfo.Capture(exn).Throw(). The helper moves from LowerAsyncSeq into RuntimeAsyncExceptionRewrite and is shared; the rewrite now receives the import map.

Fixes #20869

…suspending handlers

A try/with handler or try/finally compensation that suspends is moved out
of its exception-handling region. The rewrite recorded the body's outcome
in a Choice<'T, exn>, allocated on every run including success, and
rethrew a pending exception with a plain throw, which reset its stack
trace. This affected a suspending finally (including use of an
IAsyncDisposable), reraise () after a suspension, and an unmatched
handler pattern.

* Record the body's result and the pending exception in two locals; the
  exception stays null when the body succeeds, so nothing is allocated.
* Rethrow with ExceptionDispatchInfo.Capture(exn).Throw(). The helper moves
  from LowerAsyncSeq into RuntimeAsyncExceptionRewrite and is shared;
  the rewrite now receives the import map.
* Tests for stack traces and success-path allocation; update the IL
  fragments of the composed CE test and pin the rethrow.
* Docs and release notes.
@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

✅ Release notes checked


✅ Found changes and release notes in following paths:

Change path Release notes path Description
`src/Compiler` docs/release-notes/.FSharp.Compiler.Service/11.0.200.md

@majocha
majocha marked this pull request as ready for review October 27, 2026 14:16
@majocha
majocha requested a review from a team as a code owner October 27, 2026 14:16

@pcshrosbree pcshrosbree left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

One question about the FS3919 message for a byref-like try result. I built this PR's head (bbbaea1) and its merge base (fe180de) locally and compiled the example below with both.

Comment on lines +87 to +92
let resultVal, bodyResult =
if isUnitTy g resultTy then
None, mkUnit g m
else
let resultVal, result = mkMutableCompGenLocal m "__runtimeAsyncResult" resultTy
Some resultVal, result

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For a byref-like try result like the one below, FS3919 now names this generated local, which the user never wrote. Could the message refer to the try result instead of the generated local, or say that the result can't be held across the suspension?

let f (arr: int[]) : Task<int> =
    StateMachineHelpers.__runtimeAsyncReturn (
        let s = (try Span<int>(arr) finally AsyncHelpers.Await(Task.Delay 1))
        s.Length)

With this PR's build and --langversion:preview, this gives error FS3919: A byref, byref-like, or pinned local '__runtimeAsyncResult' cannot be used after a runtime async suspension. at the try, with optimization off and on. Rejecting it is right: the merge-base build compiles it, and on runtime 11.0.0-rc.1 f(...).Result throws an AggregateException wrapping a TypeLoadException, because Span<int> can't be the first type argument of the old Choice. None of the runtime-async tests has a byref-like try result, so a test would also keep this case rejected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch. C# in such case gives just

“Instance of type ‘System.Span‘ cannot be preserved across ‘await’ or ‘yield’ boundary.”

@T-Gro T-Gro 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

@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Nov 2, 2026
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Nov 2, 2026
@T-Gro
T-Gro self-requested a review November 2, 2026 12:22

This branch has not been deployed

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

Labels

AI-reviewed PR reviewed by AI review council

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Runtime async: rethrow after a suspending handler or finally loses the stack trace

3 participants