wasmtime / PR #11751 Cooperative Multithreading · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #11751 Cooperative Multithreading

Wasmtime GitHub notifications bot (Sep 29 2025 at 09:06):

TartanLlama opened PR #11751 from TartanLlama:sy/coop_threading to bytecodealliance:main:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[ ] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[ ] Testing

[ ] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Requires building against this branch of wit-bindgen

Wasmtime GitHub notifications bot (Sep 29 2025 at 10:46):

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

Subscribe to Label Action

cc @fitzgen

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

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 (Sep 29 2025 at 11:45):

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

Label Messager: wasmtime:config

It looks like you are changing Wasmtime's configuration options. Make sure to
complete this check list:

[ ] If you added a new Config method, you wrote extensive documentation for
it.

<details>

Our documentation should be of the following form:

```text
Short, simple summary sentence.

More details. These details can be multiple paragraphs. There should be
information about not just the method, but its parameters and results as
well.

Is this method fallible? If so, when can it return an error?

Can this method panic? If so, when does it panic?

Example

Optional example here.
```

</details>

[ ] If you added a new Config method, or modified an existing one, you
ensured that this configuration is exercised by the fuzz targets.

<details>

For example, if you expose a new strategy for allocating the next instance
slot inside the pooling allocator, you should ensure that at least one of our
fuzz targets exercises that new strategy.

Often, all that is required of you is to ensure that there is a knob for this
configuration option in [wasmtime_fuzzing::Config][fuzzing-config] (or one
of its nested structs).

Rarely, this may require authoring a new fuzz target to specifically test this
configuration. See [our docs on fuzzing][fuzzing-docs] for more details.

</details>

[ ] If you are enabling a configuration option by default, make sure that it
has been fuzzed for at least two weeks before turning it on by default.

[fuzzing-config]: https://github.com/bytecodealliance/wasmtime/blob/ca0e8d0a1d8cefc0496dba2f77a670571d8fdcab/crates/fuzzing/src/generators.rs#L182-L194
[fuzzing-docs]: https://docs.wasmtime.dev/contributing-fuzzing.html

<details>

To modify this label's message, edit the <code>.github/label-messager/wasmtime-config.md</code> file.

To add new label messages or remove existing label messages, edit the
<code>.github/label-messager.json</code> configuration file.

Learn more.

</details>

Wasmtime GitHub notifications bot (Sep 30 2025 at 08:00):

cpetig submitted PR review.

Wasmtime GitHub notifications bot (Sep 30 2025 at 08:00):

cpetig created PR review comment:

This logic looks wrong (not using p for filter_map)

Wasmtime GitHub notifications bot (Sep 30 2025 at 08:44):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Sep 30 2025 at 08:44):

TartanLlama created PR review comment:

Indeed, that's going to be fixed when I merge in the latest changes

Wasmtime GitHub notifications bot (Sep 30 2025 at 09:39):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Sep 30 2025 at 09:39):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[ ] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Requires building against this branch of wit-bindgen

Wasmtime GitHub notifications bot (Sep 30 2025 at 11:42):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Sep 30 2025 at 11:42):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[ ] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Sep 30 2025 at 11:44):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 01 2025 at 11:41):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 01 2025 at 12:10):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 03 2025 at 08:45):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 03 2025 at 13:16):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Oct 03 2025 at 16:08):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 03 2025 at 19:18):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[ ] Insert threads into the instance table rather than holding them inside GuestTask

[ ] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Oct 04 2025 at 12:40):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[x] Insert threads into the instance table rather than holding them inside GuestTask

[x] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Oct 04 2025 at 12:40):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[x] Insert threads into the instance table rather than holding them inside GuestTask

[x] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Oct 04 2025 at 12:41):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[ ] Insert threads into the instance table rather than holding them inside GuestTask

[ ] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Oct 04 2025 at 12:41):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[ ] Insert threads into the instance table rather than holding them inside GuestTask

[ ] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Oct 04 2025 at 13:04):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 04 2025 at 13:04):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[x] Insert threads into the instance table rather than holding them inside GuestTask

[x] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Oct 04 2025 at 13:04):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[x] Insert threads into the instance table rather than holding them inside GuestTask

[x] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Oct 06 2025 at 13:05):

cpetig submitted PR review.

Wasmtime GitHub notifications bot (Oct 06 2025 at 13:05):

cpetig created PR review comment:

resolved. (I don't seem to be able to close this thread)

Wasmtime GitHub notifications bot (Oct 06 2025 at 16:37):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[ ] Support for explicit threads continuing after the implicit thread has exited

[ ] Match trap conditions with the spec

[x] Insert threads into the instance table rather than holding them inside GuestTask

[x] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Oct 07 2025 at 14:24):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 07 2025 at 15:04):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 07 2025 at 15:05):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[x] Support for explicit threads continuing after the implicit thread has exited

[ ] Match trap conditions with the spec

[x] Insert threads into the instance table rather than holding them inside GuestTask

[x] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

Wasmtime GitHub notifications bot (Oct 09 2025 at 08:55):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 09 2025 at 09:08):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 09 2025 at 14:13):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 09 2025 at 14:15):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 09 2025 at 14:18):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 09 2025 at 14:30):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 09 2025 at 15:26):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 15:53):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 15:55):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 15:56):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 15:57):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 16:33):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 17:01):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 17:08):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 17:15):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 17:20):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 17:25):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 19:05):

TartanLlama has marked PR #11751 as ready for review.

Wasmtime GitHub notifications bot (Oct 10 2025 at 19:05):

TartanLlama requested alexcrichton for a review on PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 19:05):

TartanLlama requested wasmtime-fuzz-reviewers for a review on PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 19:05):

TartanLlama requested wasmtime-compiler-reviewers for a review on PR #11751.

Wasmtime GitHub notifications bot (Oct 10 2025 at 19:05):

TartanLlama requested wasmtime-core-reviewers for a review on PR #11751.

Wasmtime GitHub notifications bot (Oct 14 2025 at 18:49):

alexcrichton submitted PR review:

Some initial comments from review, mostly pretty minor. I'm ccing @dicej on this as well as he's the main author of concurrent.rs, the meat of the changes here, so he'll be taking a look at that

Wasmtime GitHub notifications bot (Oct 14 2025 at 18:49):

alexcrichton created PR review comment:

I think this, and a few updates above, may not be necessary any more? (maybe once were but may no longer be)

Wasmtime GitHub notifications bot (Oct 14 2025 at 18:49):

alexcrichton created PR review comment:

I realize that this is preexisting (my comment here applies to other tests in this file too) and so you can defer this to later if you desire, but these tests are perfect for adding to tests/misc_testsuite/**/*.wast vs in-Rust tests here. Should be able to (assert_trap (invoke ...) "the message") and that makes the tests much easier to run and avoids compiling more Rust code for tests

Wasmtime GitHub notifications bot (Oct 14 2025 at 18:49):

alexcrichton created PR review comment:

This might be an accidental update? (assuming based on the removed test)

Wasmtime GitHub notifications bot (Oct 14 2025 at 18:49):

alexcrichton created PR review comment:

In https://github.com/bytecodealliance/wasmtime/pull/11818 I'm hoping to split tests apart based on features rather than having everything in one large test, so mind adding the updated version of this test to a new test instead of updating the one here?

Wasmtime GitHub notifications bot (Oct 14 2025 at 18:49):

alexcrichton created PR review comment:

This might be another victim of a mass rename (unrelated to components here)

Wasmtime GitHub notifications bot (Oct 14 2025 at 18:49):

alexcrichton created PR review comment:

If you'd like it should be possible to reduce the number of functions here to correspond to just the suspension_intrinsic function. There's no need for the libcalls here to be 1:1 with component model libcalls as the compiler could translate, for example, the thread_suspend call here into a suspension_intrinsic libcall which would bake in false as a runtime argument in the trampoline and this function itself would take the boolean an an argument.

Mostly a stylistic preference one way or another and I don't mind either way myself. Just wanted to note the possibility in case you didn't like having so many intrinsics.

Wasmtime GitHub notifications bot (Oct 14 2025 at 18:49):

alexcrichton created PR review comment:

This might be an unrelated victim of "mass rename yield to thread-yield" (it's core modules not components)

Wasmtime GitHub notifications bot (Oct 14 2025 at 18:49):

alexcrichton created PR review comment:

Similar to my comment below, but this would also be a good test to add to the *.wast-based testing. It might be a little more verbose there but I think it'd be worth it (ideally all tests are *.wast).

Feel free to defer this to a future PR though

Wasmtime GitHub notifications bot (Oct 14 2025 at 18:49):

alexcrichton created PR review comment:

This is technically I think a victim of the mass renabme and while this is related to components it's host-level yielding as opposed to component-level yielding.

Wasmtime GitHub notifications bot (Oct 15 2025 at 16:23):

dicej submitted PR review:

Looks great; thanks for doing this!

I've only looked closely at the concurrent.rs changes; Alex said the rest looked fine. It all looks reasonable to me; just a few inline comments here and there. I appreciate the tweaks you made to the existing code to make it more readable.

Wasmtime GitHub notifications bot (Oct 15 2025 at 16:23):

dicej created PR review comment:

self doesn't appear to be used here; perhaps this function should be moved to impl ConcurrentState?

Wasmtime GitHub notifications bot (Oct 15 2025 at 16:23):

dicej created PR review comment:

    /// The "high priority" work queue for this store's event loop.
Not related to your changes, but I should have updated these docs when I moved ConcurrentState to StoreOpaque.

Wasmtime GitHub notifications bot (Oct 15 2025 at 16:23):

dicej created PR review comment:

There's a lot of unsafe code here that I don't have enough context for. @alexcrichton would you mind talking a look?

Wasmtime GitHub notifications bot (Oct 15 2025 at 16:23):

dicej created PR review comment:

This leaks information "non-determinstically" about how many table indexes have been allocated across all (sub)components by returning the rep directly. We should translate the rep to a (sub)component-specific handle using a RuntimeComponentInstanceIndex-specific table like we do for subtask, stream, future, resource, etc. handles.

Wasmtime GitHub notifications bot (Oct 15 2025 at 16:23):

dicej created PR review comment:

Optional nit: task.have_lowered_parameters() feels a bit ungrammatical. Perhaps change this to has_* instead?

Wasmtime GitHub notifications bot (Oct 15 2025 at 16:23):

dicej created PR review comment:

    /// The "low priority" work queue for this store's event loop.

Wasmtime GitHub notifications bot (Oct 15 2025 at 22:11):

dicej edited PR review comment.

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:13):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:13):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:13):

TartanLlama created PR review comment:

Change was unnecessary now, reverted

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:15):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:23):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:25):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:53):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:56):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:56):

TartanLlama created PR review comment:

Yeah, this is mostly an artifact of how the code developed, but I think it's okay to have the 1:1 correspondence as it keeps the libcalls easily searchable

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:57):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 09:57):

TartanLlama created PR review comment:

Makes sense, I'll defer for now and can address in a future PR

Wasmtime GitHub notifications bot (Oct 17 2025 at 10:02):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 17 2025 at 10:05):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 17 2025 at 10:06):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 10:06):

TartanLlama created PR review comment:

I renamed it to already_lowered_parameters because otherwise it could be interpreted as "currently owns lowered parameters", which doesn't really make sense, but I think this is less ambiguous

Wasmtime GitHub notifications bot (Oct 17 2025 at 12:25):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 12:25):

TartanLlama created PR review comment:

Ah yeah, makes sense. I've made this change, but it breaks a bunch of the existing tests that check the value of handles, e.g. https://github.com/bytecodealliance/wasmtime/blob/main/tests/misc_testsuite/component-model/async/partial-stream-copies.wast#L164

Do we consider stability of those handles part of the ABI?

Wasmtime GitHub notifications bot (Oct 17 2025 at 13:46):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 13:46):

dicej created PR review comment:

Do we consider stability of those handles part of the ABI?

The values of the handles should be a deterministic function of the specific instance's history (i.e. how many waitables, resources, etc. it creates, where some of those handles might be created implicitly). The important thing is that the history of _other_ instances (e.g. instances the original instance is composed with) should not have any effect.

Whether that deterministic function must be stable across revisions of the component model specification is an interesting question. In this case, we've added a feature that implicitly creates new handles, IIUC; in that case, it seems like we must update the deterministic function to account for that and not worry about maintaining stability before vs. after the feature was added. @lukewagner may have thoughts.

Wasmtime GitHub notifications bot (Oct 17 2025 at 14:05):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 14:05):

TartanLlama created PR review comment:

That makes sense, although my understanding is that the handle value function cannot be entirely deterministic as elements of a specific instance's execution may be non-deterministic (e.g. scheduling of yielded threads), which may affect the order in which items are added to the instance table. But that's unrelated to your main point of other instances not affecting a specific instance's execution.

As a related question, I've seen movement elsewhere to move a lot of testing up into the component model specification testing suite, but things like this are testing for implementation details of wasmtime. Is the idea that we'll still have tests that ensure observable parts of the wasmtime implementation don't change, even if the spec doesn't have an opinion on them?

Wasmtime GitHub notifications bot (Oct 17 2025 at 14:17):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 14:17):

dicej created PR review comment:

my understanding is that the handle value function cannot be entirely deterministic as elements of a specific instance's execution may be non-deterministic (e.g. scheduling of yielded threads),

Yes, good point. I think partial-stream-copies.wast is an example of avoiding such non-determinism in order to reliably predict the values of handles, but you're right that that can't be done in general.

Is the idea that we'll still have tests that ensure observable parts of the wasmtime implementation don't change, even if the spec doesn't have an opinion on them?

Per https://github.com/WebAssembly/component-model/pull/513, I think the spec _does_ have opinions about how handles are allocated, and I'd interpret e.g. partial-stream-copies.wast as testing behavior that's _not_ expected to vary across implementations. I could be wrong about that, though.

Wasmtime GitHub notifications bot (Oct 17 2025 at 14:22):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 14:22):

TartanLlama created PR review comment:

I think the spec does have opinions about how handles are allocated...

Yep, I think you're right. I'll go ahead and update those tests, thanks!

Wasmtime GitHub notifications bot (Oct 17 2025 at 15:16):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 15:16):

alexcrichton created PR review comment:

@TartanLlama mind moving this function to VMComponentConetxt since that's generally where our unsafe-frobbing bits live? Also, would it be possible to return a Func from this function instead fo a NonNull<VMFuncRef>? That'd improve the safety of this function in the sense that it's more difficult to mis-use the return value too.

Otherwise though this function should itself be safe because there's no unsafe (e.g. raw pointer) parameters and the indices should all be checked internally.

Wasmtime GitHub notifications bot (Oct 17 2025 at 15:50):

lukewagner submitted PR review.

Wasmtime GitHub notifications bot (Oct 17 2025 at 15:50):

lukewagner created PR review comment:

It's a good question (and one that we should probably consider again as we move toward 1.0, b/c there's pros/cons each way) of if and how much nondeterminism the spec should allow regarding the allocation of handles. But for now, yes, if you hold all the other nondeterministic (scheduling, et al) choices constant, handle allocation is indeed currently fully deterministic as specified by the Python "operational semantics" here. That does mean that CM/#557 is technically a "breaking" change since it adds a new element (for the implicit thread created for each export call) to the instance's table even for existing code, but I expect it won't break anything in practice so I think we're fine.

Wasmtime GitHub notifications bot (Oct 20 2025 at 14:49):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 20 2025 at 14:50):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 20 2025 at 14:50):

TartanLlama created PR review comment:

I've addressed this with this commit: https://github.com/bytecodealliance/wasmtime/pull/11751/commits/d01a51d1f8114e9d19aad82fd100f4e5d528f0d1

Wasmtime GitHub notifications bot (Oct 21 2025 at 08:06):

TartanLlama commented on PR #11751:

PR to update component model tests: https://github.com/WebAssembly/component-model/pull/570

Wasmtime GitHub notifications bot (Oct 21 2025 at 08:10):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 21 2025 at 08:41):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 21 2025 at 11:30):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 21 2025 at 11:39):

TartanLlama commented on PR #11751:

Test failures for upstream component model tests are awaiting this PR: https://github.com/WebAssembly/component-model/pull/570

Wasmtime GitHub notifications bot (Oct 21 2025 at 11:46):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 21 2025 at 11:48):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

Submitting as a draft PR for now to share progress.

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[x] Support for explicit threads continuing after the implicit thread has exited

[ ] Match trap conditions with the spec

[x] Insert threads into the instance table rather than holding them inside GuestTask

[x] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

prtest:full

Wasmtime GitHub notifications bot (Oct 21 2025 at 11:58):

TartanLlama deleted a comment on PR #11751:

PR to update component model tests: https://github.com/WebAssembly/component-model/pull/570

Wasmtime GitHub notifications bot (Oct 21 2025 at 12:03):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 21 2025 at 13:16):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 21 2025 at 13:18):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 13:18):

TartanLlama created PR review comment:

I've moved it to ComponentInstance in runtime/vm/component.rs, as that seemed a better fit that VMComponentContext, but can move it there if you think that's wrong. I've also made it return a Func and be safe.

Wasmtime GitHub notifications bot (Oct 21 2025 at 13:18):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 21 2025 at 13:20):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 21 2025 at 15:44):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 15:44):

TartanLlama created PR review comment:

This could be reverted to the old structure

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:25):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:25):

TartanLlama created PR review comment:

Rename RemoveOnDrop to something better

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:30):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:30):

TartanLlama created PR review comment:

Maybe change to sync/async stackful

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:42):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:42):

TartanLlama created PR review comment:

Factor out

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:43):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:43):

TartanLlama created PR review comment:

Store this in case the func ref changes

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:43):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:43):

TartanLlama created PR review comment:

Use cached version

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:50):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:50):

TartanLlama created PR review comment:

Create this closure inside thread_new_indirect

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:52):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:52):

TartanLlama created PR review comment:

Maybe factor out high_priority check as it's duplicated with above

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:55):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 16:55):

TartanLlama created PR review comment:

Change to a Result<Something> where Something represents Cancelled or NotCancelled

Wasmtime GitHub notifications bot (Oct 21 2025 at 17:01):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 17:01):

TartanLlama created PR review comment:

Maybe iterate once and check if the length has changed

Wasmtime GitHub notifications bot (Oct 21 2025 at 17:08):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 17:08):

TartanLlama created PR review comment:

Replace with an assertion

Wasmtime GitHub notifications bot (Oct 21 2025 at 17:10):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 21 2025 at 17:10):

TartanLlama created PR review comment:

Change to take_pending_cancellation

Wasmtime GitHub notifications bot (Oct 22 2025 at 09:30):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 22 2025 at 09:30):

TartanLlama created PR review comment:

Converted to issue: https://github.com/bytecodealliance/wasmtime/issues/11908

Wasmtime GitHub notifications bot (Oct 22 2025 at 09:33):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 22 2025 at 09:37):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 22 2025 at 10:19):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 22 2025 at 10:35):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 22 2025 at 10:38):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 22 2025 at 10:38):

TartanLlama created PR review comment:

@alexcrichton I forgot to go over this function in the call, but this is to where I moved that funcref extraction

Wasmtime GitHub notifications bot (Oct 22 2025 at 11:17):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 22 2025 at 11:19):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 22 2025 at 11:40):

TartanLlama commented on PR #11751:

Created issues #11910 #11909 #11908 to track deferred work

Wasmtime GitHub notifications bot (Oct 22 2025 at 11:42):

TartanLlama commented on PR #11751:

@alexcrichton @dicej I think we're good to go. For CI to go green, we'd need to have https://github.com/WebAssembly/component-model/pull/570 and https://github.com/WebAssembly/component-model/pull/557 merged into the component model repo. What do we think about getting them merged soon @lukewagner ?

Wasmtime GitHub notifications bot (Oct 22 2025 at 14:55):

alexcrichton created PR review comment:

For these two errors, the error message will want to be updated silghtly -- Ok(None) means that a null funcref was loaded, which technically should be plumbed to the caller as a Result<Option<Func>> return value from this function. (or document the function as "returns an error on null funcref values"). The second error, Err(e), is the out-of-bounds error and it should be fine to use ? on the get_func expression to avoid having to deal with it in the match

Wasmtime GitHub notifications bot (Oct 22 2025 at 14:55):

alexcrichton created PR review comment:

Mind using Instance::from_vmctx instead of a manual byte_sub? (that function came into existence after you wrote this I suspect)

Wasmtime GitHub notifications bot (Oct 22 2025 at 14:55):

alexcrichton created PR review comment:

For some style here, instead of using ? and/or Err(anyhow!(...)) you can use bail!(...) in the error cases

Wasmtime GitHub notifications bot (Oct 22 2025 at 14:55):

alexcrichton submitted PR review:

You can also feel free to update this function -- https://github.com/bytecodealliance/wasmtime/blob/710ba4bbf33e02f681eeada012ffa081b8d20588/crates/test-util/src/wast.rs#L404 -- to avoid blocking on PRs and go ahead and land this with known-failing tests

Wasmtime GitHub notifications bot (Oct 22 2025 at 14:55):

alexcrichton created PR review comment:

Instead of passing in &VMTableImport could this pass in a RuntimeTableIndex or similar? Otherwise this function needs to be unsafe because any table value could be passed in with invalid pointers.

Wasmtime GitHub notifications bot (Oct 22 2025 at 14:55):

alexcrichton created PR review comment:

I might recommend using offsets.ptr.vmtable_import_{vmctx,from}() here to avoid the clippy lint

Wasmtime GitHub notifications bot (Oct 22 2025 at 16:14):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Oct 22 2025 at 16:25):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 22 2025 at 16:25):

TartanLlama created PR review comment:

In that case is the documentation of get_func wrong? Current:

    /// Get reference to the specified element.
    ///
    /// Returns `None` if the index is out of bounds.
    ///
    /// Panics if this is a table of GC references and `gc_store` is `None`.

Should this instead be:

    /// Get reference to the specified element.
    ///
    /// Returns `Ok(None)` if the element is null.
    /// Returns `Err` if the index is out of bounds.
    ///
    /// Panics if this is a table of GC references and `gc_store` is `None`.

Wasmtime GitHub notifications bot (Oct 22 2025 at 16:44):

alexcrichton created PR review comment:

Ah yes good point! That is indeed outdated documentation, and your proposed docs are more accurate. The only other thing we should add is:
    /// Returns `Ok(None)` if the element is null or uninitialized.
Since Ok(None) happens for lazy-init tables where the elem hasn't been init'd yet

Wasmtime GitHub notifications bot (Oct 22 2025 at 16:44):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Oct 22 2025 at 16:54):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 22 2025 at 16:54):

TartanLlama created PR review comment:

Cool cool, I'll roll that into this PR

Wasmtime GitHub notifications bot (Oct 23 2025 at 08:49):

TartanLlama submitted PR review.

Wasmtime GitHub notifications bot (Oct 23 2025 at 08:49):

TartanLlama created PR review comment:

Hm, that seems to be a function on VMOffsets rather than PtrSize, and I think I'd need to pick a module to create a VMOffsets

Wasmtime GitHub notifications bot (Oct 23 2025 at 08:53):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 23 2025 at 09:00):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 23 2025 at 09:10):

lukewagner commented on PR #11751:

@TartanLlama That is fantastic to hear! I merged CM/#557. Do you suppose you could post a note on CM/#570 noting that it has been implemented and you think it's good to merge?

Wasmtime GitHub notifications bot (Oct 23 2025 at 09:11):

lukewagner edited a comment on PR #11751:

@TartanLlama That is fantastic to hear! I merged CM/#570. Do you suppose you could post a note on CM/#557 noting that it has been implemented and you think it's good to merge?

Wasmtime GitHub notifications bot (Oct 23 2025 at 09:14):

TartanLlama edited PR #11751:

Implements https://github.com/WebAssembly/component-model/pull/557

[x] Pass through all new builtins

[x] Basic implementations of new builtins

[x] Cancellation support

[x] Support for explicit threads continuing after the implicit thread has exited

[ ] Match trap conditions with the spec (deferred)

[x] Insert threads into the instance table rather than holding them inside GuestTask

[x] Don't reuse the existing GuestCall functionality for new threads

[ ] Testing (partially implemented, further testing deferred)

[X] Revert changes to the component_model_async_builtins and component_model_async_stackful features

prtest:full

Wasmtime GitHub notifications bot (Oct 23 2025 at 14:45):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Oct 23 2025 at 14:45):

alexcrichton created PR review comment:

Ah ok good point!

Wasmtime GitHub notifications bot (Oct 23 2025 at 19:20):

posborne submitted PR review.

Wasmtime GitHub notifications bot (Oct 23 2025 at 19:20):

posborne created PR review comment:

Looks like https://github.com/WebAssembly/component-model/pull/570 has now merged.

Wasmtime GitHub notifications bot (Oct 23 2025 at 19:33):

posborne submitted PR review.

Wasmtime GitHub notifications bot (Oct 23 2025 at 19:33):

posborne created PR review comment:

In other callsites here, access to thread state (e.g. state.get_mut(t.thread)) is done fallibly. May be worth making this consistent unless we are more confident of state presence for this specific case.

E.g.
if let Some(ref ot) = old_thread {
    state.get_mut(ot.thread)?.state = GuestThreadState::Running;
}

Wasmtime GitHub notifications bot (Oct 23 2025 at 19:53):

posborne submitted PR review:

Left a couple freestanding comments, but they are minor and can probably be deferred if relevant.

Wasmtime GitHub notifications bot (Oct 27 2025 at 10:08):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 27 2025 at 10:10):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 27 2025 at 10:13):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 27 2025 at 10:23):

TartanLlama updated PR #11751.

Wasmtime GitHub notifications bot (Oct 27 2025 at 10:25):

TartanLlama commented on PR #11751:

@alexcrichton looks like one of the tests I added was missing a Miri ignore directive which failed the merge, I've added it

Wasmtime GitHub notifications bot (Oct 27 2025 at 22:15):

alexcrichton merged PR #11751.

Last updated: Dec 06 2025 at 07:03 UTC