wasmtime / PR #12379 consistently create thread and task ... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #12379 consistently create thread and task ...

Wasmtime GitHub notifications bot (Jan 20 2026 at 22:54):

dicej requested alexcrichton for a review on PR #12379.

Wasmtime GitHub notifications bot (Jan 20 2026 at 22:54):

dicej requested wasmtime-compiler-reviewers for a review on PR #12379.

Wasmtime GitHub notifications bot (Jan 20 2026 at 22:54):

dicej requested wasmtime-core-reviewers for a review on PR #12379.

Wasmtime GitHub notifications bot (Jan 20 2026 at 22:54):

dicej opened PR #12379 from dicej:consistent-task-push to bytecodealliance:main:

Previously, we weren't creating a new thread or task in all cases when entering a component instance, even when component model async features were enabled. In particular, entering an instance via a sync-to-sync, guest-to-guest adapter, via Linker::instantiate[_async], or via [Typed]Func::call all skipped creating a thread or task, creating panics and/or instance mismatches in certain cases.

This commit addresses all those cases and also adds assertions to all CM async intrinsics to verify that the caller instance matches the most-recently-pushed task. Note that we still skip pushing and popping threads and tasks if no CM async features are enabled in the Config.

In order to populate the GuestTask::instance field for tasks created as part of Linker::instantiate[_async] calls, I had to add a RuntimeComponentInstanceIndex field to GlobalInitializer::InstantiateModule and friends so it would be available when needed.

While testing this, I uncovered and fixed a couple of related issues:

We weren't checking the may_leave flag when guest-to-guest calling a resource destructor

We weren't checking whether a subtask was ready to delete (e.g. that no threads were still running) before attempting to delete it while deleting its supertask

Wasmtime GitHub notifications bot (Jan 20 2026 at 23:01):

dicej updated PR #12379.

Wasmtime GitHub notifications bot (Jan 20 2026 at 23:08):

dicej edited PR #12379:

Previously, we weren't creating a new thread or task in all cases when entering a component instance, even when component model async features were enabled. In particular, entering an instance via a sync-to-sync, guest-to-guest adapter, via Linker::instantiate[_async], or via [Typed]Func::call all skipped creating a thread or task, leading to panics and/or instance mismatches in certain cases.

This commit addresses all those cases and also adds assertions to all CM async intrinsics to verify that the caller instance matches the most-recently-pushed task. Note that we still skip pushing and popping threads and tasks if no CM async features are enabled in the Config.

In order to populate the GuestTask::instance field for tasks created as part of Linker::instantiate[_async] calls, I had to add a RuntimeComponentInstanceIndex field to GlobalInitializer::InstantiateModule and friends so it would be available when needed.

While testing this, I uncovered and fixed a couple of related issues:

We weren't checking the may_leave flag when guest-to-guest calling a resource destructor

We weren't checking whether a subtask was ready to delete (e.g. that no threads were still running) before attempting to delete it while deleting its supertask

Wasmtime GitHub notifications bot (Jan 21 2026 at 01:05):

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

Wasmtime GitHub notifications bot (Jan 21 2026 at 01:05):

github-actions[bot] added the label wasmtime:config on PR #12379.

Wasmtime GitHub notifications bot (Jan 21 2026 at 02:30):

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

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 (Jan 22 2026 at 19:04):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:04):

alexcrichton created PR review comment:

In the interest of reducing #[cfg], could dummy versions of {enter,exit}_sync_call be added for when this feature is disabled, and they just panic? They should dynamically never be entered due to the wasm features not being enable-able so the panic should be ok

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:04):

alexcrichton created PR review comment:

Also, in retrospect, this is a huge number of checks for may_leave (preexisting) that seem easy to forget. This check is applicable to all intrinsics, right? If so in a (possibly future) PR we could update the entry trampoline for components to automatically check this

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:04):

alexcrichton created PR review comment:

For this I might recommend storing &'a Tunables in addition to the change elsewhere about passing in &Tunables instead of two bools

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:04):

alexcrichton created PR review comment:

I know I might sound like a bit of a broken record, but this, and all tests below, would make excellent *.wast tests and I believe could be copy/pasted as-is while removing boilerplate

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:04):

alexcrichton created PR review comment:

Technically this needs to happen after post-return, right? Otherwise calling intrinsics from post-return may panic?

(perhaps good tests to add?)

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:04):

alexcrichton created PR review comment:

To help optimize this better perhaps self.enabled_features.intersects(A | B |C)?

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:04):

alexcrichton created PR review comment:

I realize this is sort of the trend, but personally I'd prefer that not all fields in Wasmtime, over time, become pub(crate). Would it be possible to make an accessor for this which is easier to track and helps encapsulate this to this file? I'm also surprised that external access of this is needed since everything else should be using Tunables, not ConfigTunables

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:04):

alexcrichton created PR review comment:

This is pretty wordy in a lot of places, and one possible replacement could be instance.check_may_leave(self, caller)? maybe?

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:04):

alexcrichton created PR review comment:

Can this switch to taking &Tunables instead of two-bools?

Wasmtime GitHub notifications bot (Jan 22 2026 at 20:38):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 20:38):

dicej created PR review comment:

Sorry, this was accidentally left over from an earlier version of the code; it's not needed. I'll remove it.

Wasmtime GitHub notifications bot (Jan 22 2026 at 20:41):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 20:41):

dicej created PR review comment:

Recall that we added a constraint recently to the spec stipulating that post-return functions may not leave their instance, meaning any call to an import or instrinsic will trap.

Wasmtime GitHub notifications bot (Jan 22 2026 at 20:52):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 20:52):

dicej created PR review comment:

My first thought was to make these *.wast tests, in fact. I chose to put them here because I wanted to test the non-async APIs (i.e. Linker::instantiate and TypedFunc::call) specifically, which isn't possible via the wast crate since we switched it over to using the async APIs, correct?

Wasmtime GitHub notifications bot (Jan 22 2026 at 22:06):

alexcrichton created PR review comment:

Oh good point yeah, and yeah there's no way to do sync-plus-async in wast right now. Maybe something we should fix? Probably not here...

Wasmtime GitHub notifications bot (Jan 22 2026 at 22:06):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 22:07):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 22:07):

alexcrichton created PR review comment:

Ah, right, yes! Mind leaving a comment here to that effect?

Wasmtime GitHub notifications bot (Jan 22 2026 at 22:49):

dicej commented on PR #12379:

I've rebased this onto main and addressed the feedback so far locally. Once https://github.com/WebAssembly/component-model/pull/601 is merged, I'll push my updates.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:13):

dicej updated PR #12379.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:15):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:15):

dicej created PR review comment:

I've made it less verbose. I'll create an issue about updating the entry trampoline.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:21):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:21):

dicej created PR review comment:

https://github.com/bytecodealliance/wasmtime/issues/12397

Wasmtime GitHub notifications bot (Jan 23 2026 at 00:00):

dicej requested alexcrichton for a review on PR #12379.

Wasmtime GitHub notifications bot (Jan 23 2026 at 02:19):

alexcrichton submitted PR review:

Two other possible spots worth scrutinizing:

cabi_realloc invocations - probably don't need special treatment? Unsure.

ResourceAny::resource_drop - I think this'll need task-related treatment?

Although given the litany of entrypoints into wasm I'm finding it really hard to keep them all in sync, so it's probably also worth refactoring in theory to only have one entrypoint somewhere

Wasmtime GitHub notifications bot (Jan 23 2026 at 14:38):

dicej commented on PR #12379:

* `cabi_realloc` invocations - probably don't need special treatment? Unsure.
As with post-return functions, we don't allow cabi_realloc functions to leave the instance.
* `ResourceAny::resource_drop` - I think this'll need task-related treatment?
Yup, I'll address that and add a test.

Although given the litany of entrypoints into wasm I'm finding it really hard to keep them all in sync, so it's probably also worth refactoring in theory to only have one entrypoint somewhere

I'll open an issue for that.