wasmtime / PR #12349 refactor recursive reentrance checks · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #12349 refactor recursive reentrance checks

Wasmtime GitHub notifications bot (Jan 14 2026 at 17:39):

dicej requested alexcrichton for a review on PR #12349.

Wasmtime GitHub notifications bot (Jan 14 2026 at 17:39):

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

Wasmtime GitHub notifications bot (Jan 14 2026 at 17:39):

dicej opened PR #12349 from dicej:reentrance-refactor-part1 to bytecodealliance:main:

This commit makes a few changes related to recursive reentrance checks, instance poisoning, etc.:

Implements the more restrictive lift/lower rules described in https://github.com/WebAssembly/component-model/pull/589 such that a component instance may not lower a function lifted by one of its ancestors, nor vice-versa. Any such lower will result in a fused adapter which traps unconditionally, preventing guest-to-guest recursive reentrance without requiring data flow analysis.
- Note that this required updating several WAST tests which were violating the new rule, including some in the tests/component-model Git submodule, which I've updated.
- This is handled entirely in the fact module now; I've removed the AlwaysTrap case previously handled by wasmtime-cranelift.

Removes FLAG_MAY_ENTER from InstanceFlags. It is no longer needed for guest-to-guest calls due to the above, and for guest-to-host-to-guest calls we can rely on either FLAG_NEEDS_POST_RETURN for sync-lifted functions or the GuestTask call stack for async-lifted functions.

Adds a StoreOpaque::trapped field which is set when _any_ instance belonging to that store traps, at which point the entire store is considered poisoned, meaning no instance belonging to it may be entered. This prevents indeterminant concurrent task state left over from the trapping instance from leaking into other instances.

Note that this does _not_ include code to push and pop GuestTask instances for guest-to-guest sync-to-sync calls, nor for host-to-guest calls using e.g. the synchronous Func::call API, so certain intrinsics which expect a GuestTask to be present such as backpressure.inc will still fail in such cases. I'll address that in a later PR.

Also note that I made a small change to wasmtime-wit-bindgen, adding a Send bound on the T type parameter for store | async functions. This allowed me to recursively call {Typed}Func::call_concurrent from inside a host function, and it doesn't have any downsides AFAICT.

Fixes #12128

Wasmtime GitHub notifications bot (Jan 14 2026 at 17:39):

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

Wasmtime GitHub notifications bot (Jan 14 2026 at 18:22):

dicej updated PR #12349.

Wasmtime GitHub notifications bot (Jan 14 2026 at 19:07):

alexcrichton submitted PR review:

Minor comments here and there, but overall looks great, thanks!

Wasmtime GitHub notifications bot (Jan 14 2026 at 19:07):

alexcrichton created PR review comment:

stray refactoring leftover?

Wasmtime GitHub notifications bot (Jan 14 2026 at 19:07):

alexcrichton created PR review comment:

To confirm, this is a debugging/testing artifact and not a required part of this change?

Wasmtime GitHub notifications bot (Jan 14 2026 at 19:07):

alexcrichton created PR review comment:

You can avoid StoreComponentInstanceId::new(...).get(...) here by calling self.component_instance(instance.instance) directly I think

Wasmtime GitHub notifications bot (Jan 14 2026 at 19:07):

alexcrichton created PR review comment:

I'm surprised that existing trait implementations don't need to update with this, but I suspect that's because if the trait requires more things and then your impl requires less it's sort of a subtype relation and allowed.

Wasmtime GitHub notifications bot (Jan 14 2026 at 19:07):

alexcrichton created PR review comment:

Given the destructor-ish nature of previous_runtime_state and the mild confusion with a closure, could this be inverted as, after catch_traps, an if result.is_err() { store.0.set_trapped(); }?

Wasmtime GitHub notifications bot (Jan 14 2026 at 19:07):

alexcrichton created PR review comment:

To avoid #[cfg] here, how about putting this in ComponentStoreData and defining the methods in that file as well? Also helps to clarify that this is purely scoped to components, not applicable to core wasm.

Wasmtime GitHub notifications bot (Jan 14 2026 at 19:21):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Jan 14 2026 at 19:21):

dicej created PR review comment:

It allows me to propagate the trap in the async_round_trip_synchronous_recurse tests. I.e. it's a change needed by a tests added to cover certain host-to-guest-to-host scenarios addressed by this PR.