wasmtime / PR #11123 [DO NOT MERGE] update `component-mod... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #11123 [DO NOT MERGE] update `component-mod...

Wasmtime GitHub notifications bot (Jun 24 2025 at 16:38):

dicej opened PR #11123 from dicej:cm-async-plumbing to bytecodealliance:main:

This pulls in the latest Component Model async ABI code from the
wasip3-prototyping repo, including various API refactors and spec updates.

This includes all the changes to the wasmtime crate from wasip3-prototyping
_except_ that the concurrent submodule and child submodules contain only
non-functional stubs. For that reason, and the fact that
e.g. Func::call_async is now implemented in terms of Func::call_concurrent,
most of the component model tests are failing. This commit is not meant to be
merged as-is; a follow-up commit (to be PR'd separately) will contain the real
concurrent implementation, at which point the tests will pass again. I'm
splitting these into separate PRs to make review easier.

Note that this builds on
https://github.com/bytecodealliance/wasmtime/pull/11114; only the second commit
is new.

Wasmtime GitHub notifications bot (Jun 24 2025 at 16:39):

dicej edited PR #11123:

This pulls in the latest Component Model async ABI code from the wasip3-prototyping repo, including various API refactors and spec updates.

This includes all the changes to the wasmtime crate from wasip3-prototyping _except_ that the concurrent submodule and child submodules contain only non-functional stubs. For that reason, and the fact that
e.g. Func::call_async is now implemented in terms of Func::call_concurrent, most of the component model tests are failing. This commit is not meant to be merged as-is; a follow-up commit (to be PR'd separately) will contain the real
concurrent implementation, at which point the tests will pass again. I'm splitting these into separate PRs to make review easier.

Note that this builds on https://github.com/bytecodealliance/wasmtime/pull/11114; only the second commit is new.

Wasmtime GitHub notifications bot (Jun 24 2025 at 16:39):

dicej edited PR #11123:

This pulls in the latest Component Model async ABI code from the wasip3-prototyping repo, including various API refactors and spec updates.

This includes all the changes to the wasmtime crate from wasip3-prototyping _except_ that the concurrent submodule and child submodules contain only non-functional stubs. For that reason, and the fact that e.g. Func::call_async is now implemented in terms of Func::call_concurrent, most of the component model tests are failing. This commit is not meant to be merged as-is; a follow-up commit (to be PR'd separately) will contain the real
concurrent implementation, at which point the tests will pass again. I'm splitting these into separate PRs to make review easier.

Note that this builds on https://github.com/bytecodealliance/wasmtime/pull/11114; only the second commit is new.

Wasmtime GitHub notifications bot (Jun 24 2025 at 16:39):

dicej edited PR #11123:

This pulls in the latest Component Model async ABI code from the wasip3-prototyping repo, including various API refactors and spec updates.

This includes all the changes to the wasmtime crate from wasip3-prototyping _except_ that the concurrent submodule and child submodules contain only non-functional stubs. For that reason, and the fact that e.g. Func::call_async is now implemented in terms of Func::call_concurrent, most of the component model tests are failing. This commit is not meant to be merged as-is; a follow-up commit (to be PR'd separately) will contain the real concurrent implementation, at which point the tests will pass again. I'm splitting these into separate PRs to make review easier.

Note that this builds on https://github.com/bytecodealliance/wasmtime/pull/11114; only the second commit is new.

Wasmtime GitHub notifications bot (Jun 24 2025 at 18:38):

dicej requested alexcrichton for a review on PR #11123.

Wasmtime GitHub notifications bot (Jun 26 2025 at 22:48):

dicej updated PR #11123.

Wasmtime GitHub notifications bot (Jun 27 2025 at 15:04):

alexcrichton updated PR #11123.

Wasmtime GitHub notifications bot (Jun 27 2025 at 15:07):

alexcrichton updated PR #11123.

Wasmtime GitHub notifications bot (Jun 27 2025 at 15:08):

alexcrichton updated PR #11123.

Wasmtime GitHub notifications bot (Jun 27 2025 at 15:40):

alexcrichton updated PR #11123.

Wasmtime GitHub notifications bot (Jun 27 2025 at 15:53):

alexcrichton updated PR #11123.

Wasmtime GitHub notifications bot (Jun 27 2025 at 16:23):

alexcrichton submitted PR review:

These are a few comments so far but I've yet to really get into the meat of this. FWIW I also rebased https://github.com/bytecodealliance/wasmtime/pull/11127 on top of some of the local changes I've made, mostly to ensure that the changes don't break the full build.

Wasmtime GitHub notifications bot (Jun 27 2025 at 16:23):

alexcrichton created PR review comment:

Did this not end up getting stored in Options?

Wasmtime GitHub notifications bot (Jun 27 2025 at 16:23):

alexcrichton created PR review comment:

Are these TODOs still applicable? If so mind converting these to doc comments?

Wasmtime GitHub notifications bot (Jun 27 2025 at 16:23):

alexcrichton created PR review comment:

We'll want to smith this module a bit, mind moving this to a concurrent_disabled.rs or similar? Otherwise the big-ish changes we'll want here are:

Types in this module should somehow contain an uninhabited enum. We've got crate::runtime::Uninhabited for this purpose and that'll ensure that we never accidentally create these types.

Ideally there are no panic!s in this module. If anything takes self you can match self.uninhabited {} which is a compile-time assertion that it's not possible to be in that method.

For other methods that are panicking they'll want to return an error/trap, at least where possible.

Basically the worry is that if we have substantial #[cfg]'d-out functionality it's basically never tested in this repo so we want to make sure it's as simple as it can be an "obviously correct" for its situation. In that sense my worry here is that I don't want anything to accidentally get into a situation where a runtime panic can happen. In theory many of these methods are dynamically unreachable but it's hard to infer that just from looking at this module, so I'd feel safer transforming things to returning an error to at least know that if we are wrong and these functions are reachable that they'll return some sort of error instead of panicking.

Wasmtime GitHub notifications bot (Jun 27 2025 at 16:25):

alexcrichton commented on PR #11123:

This may not be plausible as I haven't gotten into the "meat" yet, but I'd like to pursue ideally the ability to land this PR as-is independent of https://github.com/bytecodealliance/wasmtime/pull/11127. Would it be possible to avoid changing call_async in this PR and https://github.com/bytecodealliance/wasmtime/pull/11127 would be where the implementation changes?

Wasmtime GitHub notifications bot (Jun 27 2025 at 16:27):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Jun 27 2025 at 16:27):

dicej created PR review comment:

Sorry, I don't think I understand the question. If you're wondering why the field name is prefixed with an underscore, it's because it won't be used until the full async implementation is merged. I can add a comment and/or #[cfg(allow(dead_code))] if that helps.

Wasmtime GitHub notifications bot (Jun 27 2025 at 16:28):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Jun 27 2025 at 16:28):

dicej created PR review comment:

Yes, they're still applicable. I'll convert them.

Wasmtime GitHub notifications bot (Jun 27 2025 at 18:49):

github-actions[bot] commented on PR #11123:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Jun 27 2025 at 20:19):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Jun 27 2025 at 20:19):

dicej created PR review comment:

I started changing the panics into compile-time assertions or traps, but I quickly realized that in almost all cases the code really should be unreachable, since any attempt to use CM async features should have failed at wasmparser validation time. It feels misleading to me to trap in those cases, because it implies that the code _is_ reachable, i.e. that a guest would somehow sneak use of an async feature past the validator.

I'll go ahead and switch to traps, but also add comments indicating that it's our (Wasmtime's) fault if they're hit, not the guest's.

Wasmtime GitHub notifications bot (Jun 27 2025 at 20:31):

dicej commented on PR #11123:

Would it be possible to avoid changing call_async in this PR and #11127 would be where the implementation changes?

I'll take a shot at it.

Wasmtime GitHub notifications bot (Jun 27 2025 at 20:44):

dicej updated PR #11123.

Wasmtime GitHub notifications bot (Jun 27 2025 at 21:04):

dicej updated PR #11123.

Last updated: Dec 06 2025 at 06:05 UTC