Stream: git-wasmtime

Topic: wasmtime / PR #13708 Trap on sync stream/future cancel an...


view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 17:54):

GodlyDonuts opened PR #13708 from GodlyDonuts:async-cancel-waitable-set-trap to bytecodealliance:main:

Fixes #13690.

[component-model#647] tightened the trap rules for synchronous stream/future
operations: a synchronous read/write must trap if the waitable it operates
on has already been added to a waitable set. That PR extended the same rule to
{stream,future}.cancel-{read,write} and subtask.cancel, but Wasmtime only
implemented it for read/write. As a result the upstream
trap-if-sync-and-waitable-set.wast test hits an unreachable on the cancel
cases today instead of trapping.

Why it was missing

The check lives in wait_for_event, the single point every blocking
synchronous read/write funnels through before suspending:

if waitable.common(state)?.set.is_some() {
    bail!(Trap::WaitableSyncAndAsync);
}

The cancel paths don't reliably reach it. When there is nothing pending to wait
on, cancel_read/cancel_write take an early return with a Cancelled code,
and subtask.cancel can resolve a starting/finished subtask without ever
suspending. In those cases the waitable-set check was never run, so the cancel
succeeded where the spec requires a trap.

Fix

Pull the check into a small helper on Waitable:

fn trap_if_in_waitable_set(&self, state: &mut ConcurrentState) -> Result<()> {
    if self.common(state)?.set.is_some() {
        bail!(Trap::WaitableSyncAndAsync);
    }
    Ok(())
}

wait_for_event now calls it (no behavior change), and the three synchronous
cancel entry points — cancel_read, cancel_write, and subtask_cancel
call it up front, guarded by !async_ to match the neighbouring
check_blocking guards. The async variants are deliberately untouched: being a
member of a waitable set is the whole point of an asynchronous cancel.

Testing

Added tests/misc_testsuite/component-model/async/cancel-sync-and-waitable.wast
covering all five operations (future.cancel-{read,write},
stream.cancel-{read,write}, and subtask.cancel). Each starts a pending
operation, joins the waitable to a fresh set, and asserts the synchronous
cancel traps; this mirrors the upstream trap-if-sync-and-waitable-set.wast. I
verified each case regresses (returns a cancel code / completes normally rather
than trapping) when its individual guard is removed.

cargo test --test wast --features component-model-async and the cancellation
integration tests in tests/all/component_model/async.rs both pass.

[component-model#647]: https://github.com/WebAssembly/component-model/pull/647

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 17:54):

GodlyDonuts requested cfallin for a review on PR #13708.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 17:54):

GodlyDonuts requested wasmtime-core-reviewers for a review on PR #13708.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 18:03):

cfallin unassigned cfallin from PR #13708 Trap on sync stream/future cancel and subtask.cancel when in a waitable set.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 18:03):

cfallin requested dicej for a review on PR #13708.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 18:05):

cfallin commented on PR #13708:

I'm going to redirect review to someone who knows much more about the CM async machinery (hope that's ok @dicej!).

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 18:50):

GodlyDonuts edited PR #13708:

Fixes #13690.

[component-model#647] tightened the trap rules for synchronous stream/future
operations: a synchronous read/write must trap if the waitable it operates
on has already been added to a waitable set. That PR extended the same rule to
{stream,future}.cancel-{read,write} and subtask.cancel, but Wasmtime only
implemented it for read/write. As a result the upstream
trap-if-sync-and-waitable-set.wast test hits an unreachable on the cancel
cases today instead of trapping.

Why it was missing

The check lives in wait_for_event, the single point every blocking
synchronous read/write funnels through before suspending:

if waitable.common(state)?.set.is_some() {
    bail!(Trap::WaitableSyncAndAsync);
}

The cancel paths don't reliably reach it. When there is nothing pending to wait
on, cancel_read/cancel_write take an early return with a Cancelled code,
and subtask.cancel can resolve a starting/finished subtask without ever
suspending. In those cases the waitable-set check was never run, so the cancel
succeeded where the spec requires a trap.

Fix

Pull the check into a small helper on Waitable:

fn trap_if_in_waitable_set(&self, state: &mut ConcurrentState) -> Result<()> {
    if self.common(state)?.set.is_some() {
        bail!(Trap::WaitableSyncAndAsync);
    }
    Ok(())
}

wait_for_event now calls it (no behavior change), and the three synchronous
cancel entry points — cancel_read, cancel_write, and subtask_cancel
call it up front, guarded by !async_ to match the neighbouring
check_blocking guards. The async variants are deliberately untouched: being a
member of a waitable set is the whole point of an asynchronous cancel.

Testing

Added tests/misc_testsuite/component-model/async/cancel-sync-and-waitable.wast
covering all five operations (future.cancel-{read,write},
stream.cancel-{read,write}, and subtask.cancel). Each starts a pending
operation, joins the waitable to a fresh set, and asserts the synchronous
cancel traps; this mirrors the upstream trap-if-sync-and-waitable-set.wast. I
verified each case regresses (returns a cancel code / completes normally rather
than trapping) when its individual guard is removed.

Depends on bytecodealliance/wit-bindgen#1638

Enforcing this trap surfaces a latent bug in wit-bindgen's async runtime:
cancel_inter_task_stream_read issues a synchronous stream.cancel-read on its
inter-task wakeup stream while that stream is still in the task's waitable set
(it calls remove_waitable only afterwards). That is exactly the pattern this
trap now (correctly) rejects, so every wasi:http@0.3.0 (p3) program built with
the current wit-bindgen traps during ordinary operation — which is what the
red p3 CI jobs here are.

bytecodealliance/wit-bindgen#1638 reorders that to leave the set before
cancelling, matching the unregister-then-cancel ordering the general
WaitableOperation::cancel path already uses. With that change applied locally
(via [patch.crates-io]), the full p3 suites pass again:

So this PR needs a wit-bindgen release containing #1638 and a corresponding
version bump before its p3 jobs go green. Happy to fold the bump in here once
that's available, or sequence it however you prefer.

[component-model#647]: https://github.com/WebAssembly/component-model/pull/647

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 20:51):

:thumbs_up: dicej submitted PR review:

LGTM, thanks!

CI is failing, and it seems to point to wit_bindgen::rt's internal machinery for handling inter-task communication. I suspect a change will be needed in that project in order to get those tests passing. I'm on vacation at the moment (and should not have opened my computer at all :sweat_smile: ), but I can take a closer look when I get back if nobody beats me to it.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 21:05):

GodlyDonuts commented on PR #13708:

Thanks @dicej! That matches what I found — the trap fires in wit-bindgen's cancel_inter_task_stream_read, which issues a synchronous stream.cancel-read on the inter-task wakeup stream while it's still in the task's waitable set (it calls remove_waitable only afterwards).

I opened bytecodealliance/wit-bindgen#1638 to fix that: it reorders the two so the stream leaves the waitable set before the synchronous cancel, matching the unregister-then-cancel ordering the general WaitableOperation::cancel path already uses. With that patched in locally ([patch.crates-io]), the full p3 suites pass again — wasmtime-wasi-http p3 (25), wasmtime-wasi p3 (27), and component-model/async wast incl. the new test (192).

So this just needs a wit-bindgen release containing #1638 and the corresponding bump here. Happy to fold the bump into this PR once it's available — no rush, enjoy your vacation. :slight_smile:

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 21:06):

GodlyDonuts edited a comment on PR #13708:

Thanks @dicej! That matches what I found, the trap fires in wit-bindgen's cancel_inter_task_stream_read, which issues a synchronous stream.cancel-read on the inter-task wakeup stream while it's still in the task's waitable set (it calls remove_waitable only afterwards).

I opened bytecodealliance/wit-bindgen#1638 to fix that: it reorders the two so the stream leaves the waitable set before the synchronous cancel, matching the unregister-then-cancel ordering the general WaitableOperation::cancel path already uses. With that patched in locally ([patch.crates-io]), the full p3 suites pass again, wasmtime-wasi-http p3 (25), wasmtime-wasi p3 (27), and component-model/async wast incl. the new test (192).

So this just needs a wit-bindgen release containing #1638 and the corresponding bump here. Happy to fold the bump into this PR once it's available, no rush, enjoy your vacation. :slight_smile:

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2026 at 22:20):

github-actions[bot] added the label wasmtime:api on PR #13708.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 24 2026 at 16:25):

GodlyDonuts requested dicej for a review on PR #13708.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 24 2026 at 16:29):

GodlyDonuts commented on PR #13708:

wit-bindgen#1638 is merged, thanks @alexcrichton. Only thing left here is a wit-bindgen release that carries it plus a matching version bump in this PR. I'll put the bump up as soon as there's something on crates.io to point at. Verified locally that the p3 suites go green with the fix in, so CI should be clean once it's bumped.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 07 2026 at 21:34):

alexcrichton commented on PR #13708:

It's taken a bit of time (apologies and thanks for the patience @GodlyDonuts), but I've got a bump for wit-bindgen over at https://github.com/bytecodealliance/wasmtime/pull/13844. Once that's merged could you rebase this and then I'd be happy to r+ and merge

view this post on Zulip Wasmtime GitHub notifications bot (Jul 07 2026 at 21:58):

GodlyDonuts commented on PR #13708:

sounds good to me

view this post on Zulip Wasmtime GitHub notifications bot (Jul 07 2026 at 22:47):

:thumbs_up: dicej submitted PR review:

LGTM, thanks!

CI is failing, and it seems to point to wit_bindgen::rt's internal machinery for handling inter-task communication. I suspect a change will be needed in that project in order to get those tests passing. I'm on vacation at the moment (and should not have opened my computer at all :sweat_smile: ), but I can take a closer look when I get back if nobody beats me to it.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 07 2026 at 22:47):

GodlyDonuts updated PR #13708.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 07 2026 at 22:59):

GodlyDonuts commented on PR #13708:

It's taken a bit of time (apologies and thanks for the patience @GodlyDonuts), but I've got a bump for wit-bindgen over at #13844. Once that's merged could you rebase this and then I'd be happy to r+ and merge

done

view this post on Zulip Wasmtime GitHub notifications bot (Jul 07 2026 at 23:13):

alexcrichton commented on PR #13708:

Thanks! I think the CI failure here is legitimate, and might require some small tweaks to the test?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 07 2026 at 23:57):

dicej commented on PR #13708:

I can take a look at that tomorrow if nobody beats me to it.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 08 2026 at 14:10):

dicej updated PR #13708.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 08 2026 at 14:22):

alexcrichton added PR #13708 Trap on sync stream/future cancel and subtask.cancel when in a waitable set to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 08 2026 at 14:45):

:check: alexcrichton merged PR #13708.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 08 2026 at 14:45):

alexcrichton removed PR #13708 Trap on sync stream/future cancel and subtask.cancel when in a waitable set from the merge queue.


Last updated: Jul 29 2026 at 05:03 UTC