Skip to content

C# async fixes - #1747

Merged
alexcrichton merged 4 commits into
bytecodealliance:mainfrom
phillipcaofph:csharp-async-fixes
Oct 15, 2026
Merged

C# async fixes#1747
alexcrichton merged 4 commits into
bytecodealliance:mainfrom
phillipcaofph:csharp-async-fixes

Conversation

@phillipcaofph

@phillipcaofph phillipcaofph commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes the following issues:

  • An async function returning result (no payload) was declared Task
  • The export wrapper didn't await the export, then awaited the Result value
  • Async imports called a class IImportsInterop that doesn't exist
  • Futures and streams were numbered separately in import names; the counter was also per-interface, not per-function
  • Futures in an async import's result got an empty function name in their import names
  • Any struct, including Result<,>, was pinned as a raw buffer
  • Native buffers were sized for one element
  • Reads called the lift function even when there is none (primitive payloads)
  • An export that failed showed up as a NullReferenceException

@phillipcaofph
csharp: build test projects with wasi-sdk 34 and on linux-arm64
53a720c 
wasi-sdk 34's wasm-ld puts the stack first by default, which needs the
data to start above the stack, but the ILCompiler sets a global base of
1024, so every C# test failed to link. Put the stack after the data
instead: raising the global base to the stack size links, but breaks the
runtime's exception unwinding.

Also reference the ILCompiler runtime package for the host architecture
rather than always x64, so the tests run on aarch64 Linux.
@phillipcaofph
csharp: fix component-model async codegen
8229e8c 
- Declare an async function returning `result` without payloads as
  `Task`, not `Task<void>`.
- Await the export in the async-lift wrapper and wrap its value in the
  Result, instead of calling it unawaited and awaiting the Result.
- Call async imports through the interface's interop class rather than
  a nonexistent `IImportsInterop`.
- Number futures and streams together, in signature order, in the
  `[future-*-N]` / `[stream-*-N]` intrinsic names, as the canonical ABI
  does. A function with a stream parameter and a future result (such as
  `wasi:cli/stdout.write-via-stream`) produced an invalid component.
- Name and number futures and streams in an async import's result
  after the function, instead of an empty function name.
- Pin only primitive future/stream buffers. Other payloads, such as
  `Result<,>`, hold references or differ from the canonical ABI layout,
  so they go through native memory and the vtable's Lift. Size native
  buffers for the whole read, and don't call a missing Lift.
- Rethrow an async export's failure before its first suspension instead
  of reporting a NullReferenceException, fail fast on a later failure,
  and explain an export waiting on something that is not a waitable.
- Fully qualify Task, List, Action and TaskContinuationOptions so the
  generated code compiles without ImplicitUsings.

Adds runtime tests for async exports returning results and for a
stream parameter with a future result.
@alexcrichton

alexcrichton commented Oct 15, 2026

Copy link
Copy Markdown
Member

Thanks! We don't have a ton of active C# reviewers at this time and I don't know enough myself to be able to review this personally. That being said at a skim this all seems fine, and it's ok to land code like this in this repo in general.

While you're at it though, would you be up for helping out figuring out how to get C# re-enabled in CI?

@phillipcaofph
csharp: fix async import result buffers and stream value lifting
e2e1bc5 
- Size the buffer an async import writes its result into from the
  result type's canonical ABI layout. It was 4 bytes for any result, so
  a string or record result overran it, and larger flat results hit a
  todo!() in the generator.
- Free that buffer with AlignedFree once the call's task completes. It
  was freed with Free right after starting the call, so a call that was
  still pending wrote and lifted its result from freed memory.
- Don't throw from LiftingTaskCompletionSource after setting a result.
- Pass the number of values read to a stream's Lift and lift each of
  them, instead of only the first.
- Release a primitive stream read's pinned buffer after a successful
  read; it took the faulted branch and skipped its cleanups.

Adds a runtime test with async imports that return a string and a
record while still pending, and a multi-value stream<string> read.
@phillipcaofph
ci: re-enable C# testing
26820bd 
C# was removed from the test matrix when moving it from wasi-sdk 29 to
34, whose linker rejected the ILCompiler's memory layout; the test
harness now links with --no-stack-first. Also install .NET 10, which the
generated test projects target.

Fix the one C# codegen test failure, issue1703.wit, added while C# was
out of CI: a variant case named `tag` or `tags` generated a factory
method clashing with the variant's `Tag` field or `Tags` class. Those
cases now get the same `_` suffix as a case named after its variant.
@phillipcaofph phillipcaofph changed the title Csharp async fixes C# async fixes Oct 15, 2026
@phillipcaofph

phillipcaofph commented Oct 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@alexcrichton I just noticed this PR was accidentally created, but thanks for looking at it.

I've added a couple of other fixes since your review and enabled the CI as you've requested :)

alexcrichton
alexcrichton approved these changes Oct 15, 2026
View reviewed changes

@alexcrichton alexcrichton 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.

Thanks!

@alexcrichton
alexcrichton added this pull request to the merge queue Oct 15, 2026
Merged via the queue into bytecodealliance:main with commit 4f7496b Oct 15, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexcrichton alexcrichton alexcrichton approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@phillipcaofph @alexcrichton