Refactor borrow state tracking for async tasks (#12550)

* Refactor borrow state tracking for async tasks

This commit is a somewhat deep refactoring of how the state of
`borrow<T>` is managed for b

Refactor borrow state tracking for async tasks (#12550)

* Refactor borrow state tracking for async tasks

This commit is a somewhat deep refactoring of how the state of
`borrow<T>` is managed for both the host and the guest with respect to
async tasks. This additionally refactors how some async task management
is done for host-called functions.

The fundamental problem being tackled here is #12510. In that issue it
was discovered that the way `CallContext`, the borrow tracking mechanism
in Wasmtime, is managed is incompatible with async tasks. Specifically
the previous assumption of the scope being mutated for a borrow is
somewhere on the call stack is no longer true. It's possible for an
async task to be suspended, for example, and then a sibling task drops a
borrow which should update the scope of the suspended task. There were a
number of other small issues I noticed here and there which this PR
additionally has tests for, all of which failed before this change and
pass afterwards.

The manner in which borrow state is manipulated is a pretty old part of
the component model implementation dating back to the original
implementation of resources. I decided to forgo any possible quick fix
and have attempted to more deeply refactor and integrate async tasks
into all of this infrastructure. A list of the changes made here are:

* The `CallContexts` structure, a stack of `CallContext`, was removed.
Tasks now directly store a `CallContext` which is the source of truth
for borrow tracking for that call, and it does not move from this
location. The store `CallContexts` is now deleted in favor of updating
the `Option<ConcurrentState>` in the store to be an `enum` of either
concurrent state or a stack. In this manner the old stack-based
structure is still used sometimes, but it's impossible to reach when
concurrency is enabled.

* Entry to the host from guests now reliably pushes a `HostTask` into
the store. Previously where a frame were always pushed into a
`CallContext` a `HostTask` is pushed into the store. This is still
expected to be a bit too expensive for cheap host calls, but it
doesn't meaningfully change the performance profile of before.

* The `resource_enter_call` and `resource_exit_call` libcalls have been
removed. These are now folded into the `enter_sync_call` and
`exit_sync_call` libcalls. Emission of these hooks has been updated
accordingly. The concept of entering a call more generally has been
removed. This is more formally known in the async world as a task
starting, so the task creation is now responsible for the demarcation
of entering a call. Additionally this means that the concept of
exiting a call has somewhat gone away. Instead this method was renamed
to `validate_scope_exit` which double-checks that a borrow-scope can
be exited but doesn't actually remove the task. Task removal is
deferred to preexisting mechanisms.

* Management of a `GuestTask`'s previous `Option<CallContext>` field,
for example taking/restoring and pushing/popping onto `CallContexts`
is now all gone. All related code is outright deleted as the
`GuestTask`'s now non-optional `CallContext` field is the source of truth.

* The `ConcurrentState` structure now stores a `CurrentThread` enum
instead of `Option<QualifiedThreadId>`. This represents how the
currently executing thread could be a host thread, not just a guest
thread, which is required for borrow-tracking.

* `HostTask` creation in `poll_and_block` and `first_poll`, the two main
entrypoints of async host tasks when called by the guest, is now
externalized from these functions. Instead these functions assume that
the currently running thread is already a `HostTask` of some kind.

* In `poll_and_block` the host's result is no longer stored in the guest
task but in the host task instead.

Overall this enables the `*.wast` test for #12510 to fix the original
issue. This then adds new tests to ensure that cleanup of various
constructs happens appropriately, such as cancelling a host task should
clean up its associated resources. Additionally synchronously calling an
async host task no longer leaks resources in a `Store` and should
properly clean up everything.

There is still more work to do in this area (e.g. #12544) but that's
going to be deferred to a future PR at this point.

Closes #12510

prtest:full

* Review comments/CI fixes

show more ...

Move threads to their own index space in components (#12516)

* Move threads to their own index space in components

This accounts for WebAssembly/component-model#600 and mostly updates
various test

Move threads to their own index space in components (#12516)

* Move threads to their own index space in components

This accounts for WebAssembly/component-model#600 and mostly updates
various test expectations to match this.

Closes #11954

* Fix conditional compiles

show more ...

Check may-leave flags in trampolines, not Rust (#12427)

This commit moves all may-leave flag handling into compiled trampolines
rather than doing this in Rust. This means it can't be forgotten on th

Check may-leave flags in trampolines, not Rust (#12427)

This commit moves all may-leave flag handling into compiled trampolines
rather than doing this in Rust. This means it can't be forgotten on the
Rust side of things and will be slightly more efficient to boot. This
then additionally exempts some intrinsics from checking may-leave since
Wasmtime erroneously checked when it shouldn't have.

Closes #12397
Closes #12403

show more ...

deprecate `[Typed]Func::post_return[_async]` and make them no-ops (#12498)

* deprecate `[Typed]Func::post_return[_async]` and make them no-ops

With the advent of the Component Model concurrency ABI

deprecate `[Typed]Func::post_return[_async]` and make them no-ops (#12498)

* deprecate `[Typed]Func::post_return[_async]` and make them no-ops

With the advent of the Component Model concurrency ABI and it's `task.return`
intrinsic, post-return functions have been informally deprecated and are
expected to be removed for WASI 1.0 and the corresponding stable edition of the
Component Model. Consequently, it does not make sense anymore to require
embedders to explicitly call the post-return function after using
`[Typed]Func::call[_async]`.

As of this commit, `[Typed]Func::post_return[_async]` are no-ops. Instead, the
post-return function is called automatically as part of
`[Typed]Func::call[_async]` if present, which is how
`[Typed]Func::call_concurrent` has worked all along. In addition, this commit
fixes and tests a couple of cases where the task and/or thread was being
disposed of before the post-return function was called.

* address review feedback

* test post-return function in more scenarios

Specifically, I've split the `invoke_post_return` test into multiple tests:

- using `TypedFunc::call`
- using `TypedFunc::call_async` with concurrency support enabled
- using `TypedFunc::call_async` with concurrency support disabled
- using `Func::call_async` with concurrency support disabled
- using `TypedFunc::call_concurrent`

* remove GCC/clang-specific deprecation attribute

This broke the MSVC build.

* bless bindgen output

* remove obsolete post-return functions and fields

Now that post-return calls are handled internally without requiring explicit
action by the embedder, we can avoid unnecessary bookkeeping.

show more ...

Introduce `wasmtime::Store::try_new`, which handles OOM (#12415)

* Introduce `wasmtime::Store::try_new`, which handles OOM

`Store::new` is an infallible constructor, so there is not a direct way to

Introduce `wasmtime::Store::try_new`, which handles OOM (#12415)

* Introduce `wasmtime::Store::try_new`, which handles OOM

`Store::new` is an infallible constructor, so there is not a direct way to make
it return an error on OOM. Additionally, it is one of the most-used functions in
the Wasmtime embedder API, so changing its signature to return a `Result` is a
non-starter -- it would cause way too much pain. So instead we define
`Store::try_new` which returns a `Result` and make `Store::new` call and unwrap
that new constructor.

Part of https://github.com/bytecodealliance/wasmtime/issues/12069

* update disas tests and fix winch

* Disable concurrency support in `Store::try_new` OOM test

* Add attributes that were lost in rebase

show more ...

refactor recursive reentrance checks (#12349)

* refactor recursive reentrance checks

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

- Implements

refactor recursive reentrance checks (#12349)

* refactor recursive reentrance checks

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

* bless bindgen expansions

* bless disas tests

* address review feedback

* sync `trap.h` with `trap_encoding.rs`

...and add const assertions to `trap.rs` to help avoid future divergence.

show more ...

add missing may-block checks for sync-to-sync guest-to-guest calls (#12282)

* add missing may-block checks for sync-to-sync guest-to-guest calls

Previously, we weren't updating or checking the may-

add missing may-block checks for sync-to-sync guest-to-guest calls (#12282)

* add missing may-block checks for sync-to-sync guest-to-guest calls

Previously, we weren't updating or checking the may-block status of a task
across sync-to-sync, guest-to-guest calls, meaning we were allowing blocking in
cases we shouldn't have.

This fixes that by adding a new `task_may_block` field to `VMComponentContext`,
plus code to update it every time we switch threads or do a sync-to-sync,
guest-to-guest call. We use that field as the source of truth about whether a
blocking operation is permitted.

I've updated various tests to match, and Luke has an item on his to-do list to
add sad-path coverage for various cases to the upstream `component-model` test
suite.

* address review feedback and fix component_instance_size_limit test

* remove `TaskMayBlock` type per review feedback

* bless disas tests

show more ...

Migrate the `wasmtime` crate to `wasmtime_environ::error::*` (#12231)

* Migrate the `wasmtime` crate to `wasmtime_environ::error::*`

Instead of `anyhow::Error`.

This commit re-exports the `wasmtim

Migrate the `wasmtime` crate to `wasmtime_environ::error::*` (#12231)

* Migrate the `wasmtime` crate to `wasmtime_environ::error::*`

Instead of `anyhow::Error`.

This commit re-exports the `wasmtime_environ::error` as the `wasmtime::error`
module, updates the prelude to include these new error-handling types, redirects
our top-level `wasmtime::{Error, Result}` re-exports to re-export
`wasmtime::error::{Error, Result}`, and updates various use sites that were
directly using `anyhow` to use the new `wasmtime` versions.

This process also required updating the component macro and wit-bindgen macro to
use the new error types instead of `anyhow`.

Part of https://github.com/bytecodealliance/wasmtime/issues/12069

* Replace wasmtime::error::Thing with wasmtime::Thing where it makes sense

* cargo fmt

* Move `crate::error::Thing` to `crate::Thing` where it makes sense

show more ...

allow recursive Wasm invocation from concurrent host functions (#12152)

* allow recursive Wasm invocation from concurrent host functions

The core changes here are:

- remove an unnecessary assertio

allow recursive Wasm invocation from concurrent host functions (#12152)

* allow recursive Wasm invocation from concurrent host functions

The core changes here are:

- remove an unnecessary assertion from `concurrent::prepare_call`
- track instance states (e.g. backpressure, etc.) on a per `(Instance, RuntimeComponentInstanceIndex)` basis
- both parts of that key are needed now that concurrent state is tracked on a per-store basis rather than a per-instance basis since `RuntimeComponentInstanceIndex`es are not globally unique

While discussing the above with Alex, we realized the use of a `HashMap` to
track per-instance states was both pessimal and unnecessary. Consequently, I've
folded that state into `ComponentInstance::instance_handle_tables`, renaming it
to `instance_states`. That involved a fair amount of code churn, but doesn't
change behavior except as described in the second bullet point above.

Thanks to Alex for the test case!

Fixes #12098

Co-authored-by: Alex Crichton <[email protected]>
Signed-off-by: Joel Dice <[email protected]>

* use new `RuntimeInstance` type instead of tuples

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>
Co-authored-by: Alex Crichton <[email protected]>

show more ...

Deduplicate static/dynamic host function code paths (#12146)

* Deduplicate static/dynamic host function code paths

This commit refactors the `component/func/host.rs` file to deduplicate
the paths b

Deduplicate static/dynamic host function code paths (#12146)

* Deduplicate static/dynamic host function code paths

This commit refactors the `component/func/host.rs` file to deduplicate
the paths between static/dynamic host functions. Previously there was a
significant amount of duplication between the two which has been
exacerbated through time. This commit refactors the state of affairs to
ensure that all the shared logic between the two is in one location and
the only difference is what they're already doing different (e.g.
lifting/lowering guts).

The high-level goal here was to see if this was possible, but in the end
this feels like a much cleaner state of affairs than prior as far fewer
details are duplicated across a few locations. The host function
behavior is slightly more "dynamic" than before in the sense that
statically-known signature has a few more type lookups than before, for
example, but this can be fixed in due time if necessary.

* Fix compile warnings/issues

show more ...

Debug: create private code memories per store when debugging is enabled. (#12051)

* Debug: create private code memories per store when debugging is enabled.

This will allow patching code to implem

Debug: create private code memories per store when debugging is enabled. (#12051)

* Debug: create private code memories per store when debugging is enabled.

This will allow patching code to implement e.g. breakpoints. (That is,
for now the copies are redundant, but soon they will not be.)

This change follows the discussion [here] and offline to define a few
types that better encapsulate the distinction we want to enforce.
Basically, there is almost never a bare `CodeMemory`; they are always
wrapped in an `EngineCode` or `StoreCode`, the latter being a per-store
instance of the former. Accessors are moved to the relevant place so
that, for example, one cannot get a pointer to a Wasm function's body
without being in the context of a `Store` where the containing module
has been registered. The registry then returns a `ModuleWithCode` that
boxes up a `Module` reference and `StoreCode` together for cases where
we need both the metadata from the module and the raw code to derive
something.

The only case where we return raw code pointers to the `EngineCode`
directly have to do with Wasm-to-array trampolines: in some cases, e.g.
`InstancePre` pre-creating data structures with references to host
functions, it breaks our expected performance characteristics to make
the function pointers store-specific. This is fine as long as the
Wasm-to-array trampolines never bake in direct calls to Wasm functions;
the strong invariant is that Wasm functions never execute from
`EngineCode` directly. Some parts of the component runtime would also
have to be substantially refactored if we wanted to do away with this
exception.

The per-`Store` module registry is substantially refactored in this PR.
I got rid of the modules-without-code distinction (the case where a
module only has trampolines and no defined functions still works fine),
and organized the BTreeMaps to key on start address rather than end
address, which I find a little more intuitive (one then queries with the
dual to the range -- 0-up-to-PC and last entry found).

[here]: https://github.com/bytecodealliance/wasmtime/pull/12051#pullrequestreview-3493711812

* Review feedback: do not assume a reasonable code alignment; error when it cannot be known

* Review feedback: fail properly in profiler when we are cloning code

* Fix guest-profiler C API.

* Review feedback: make private-code representation impossible in non-debugging-support builds.

* Add TODO comment referencing issue for cloning only .text.

* clang-format

* Review feedback: add back Component::image_range.

* Review feedback: error on registering profiling metadata when debug is enabled.

* rustfmt

* Remove early bail on profiling-data registration when debugging is enabled: this always happens so we cannot error out.

show more ...

Enable borrowing components and stores mutable at the same time (#11987)

* Use `OptionsIndex` more internally in components

This updates various runtimes bits for components to use `OptionsIndex`

Enable borrowing components and stores mutable at the same time (#11987)

* Use `OptionsIndex` more internally in components

This updates various runtimes bits for components to use `OptionsIndex`
more aggressively and ultimately deletes the old `Options` type. The
`Options` type is a heavyweight package of all possible options which is
effectively a duplication of what `OptionsIndex` points to, so that's
removed in favor of directly accessing options.

* Enable borrowing components and stores mutable at the same time

This commit adds a new, safe, function to the `Instance` type for
internal use in Wasmtime which enables simultaneously borrowing the
`Component`-within-the-`Instance` as well as the original store at the
same time. This is not possible to do in safe Rust when the store is
mutable and must be implemented with `unsafe` code.

The motivation for this commit is performance. In #11974 it was
discovered that the addition of a single `Arc::clone` was enough to
reduce throughput in the embedding by 20%. While `Arc::clone` is
generally cheap I can see how a "contended" `Arc::clone` could get quite
expensive. This particular benchmark was running multiple instances of
the same component across multiple threads which were all doing many
host calls. Each host call, after the refactoring of #10959, contained
an `Arc::clone` to the component itself. This in turn led to many
threads constantly incrementing/decrementing the `Arc::clone` count of
the same `Arc` instance.

At a high level this clone is not necessary. The component lives within
the store and cannot be mutated/deleted during execution. Safe Rust,
however, forbids access to the component and mutably using the store at
the same time. Thus, this helper function enters the picture. The goal
here is to remove the `Arc::clone` calls without requiring major surgery
or such to refactor the implementations of various functions throughout
Wasmtime. The `unsafe` block used to implement this function documents
more rationale as to why this should be safe.

* Update crates/wasmtime/src/runtime/component/instance.rs

Co-authored-by: Nick Fitzgerald <[email protected]>

---------

Co-authored-by: Nick Fitzgerald <[email protected]>

show more ...

Cooperative Multithreading (#11751)

* Initial work

* Almost compiling

* Partially working

* Cleanup

* Fix merge

* Cancellation and suspension refactoring

* Remove printlns

* Test with several

Cooperative Multithreading (#11751)

* Initial work

* Almost compiling

* Partially working

* Cleanup

* Fix merge

* Cancellation and suspension refactoring

* Remove printlns

* Test with several threads

* More testing

* Cancellation

* Fix cancellation for explicit suspends

* Finish cancellation test

* Store threads in the instance table

* Deletion almost there

* Tests all pass

* Tighten up task deletion

* Set thread state correctly

* Store pairs of thread and task ids

* Remove lift abi members

* Cleanup unnecessary change

* More cleanup

* Cleanup

* Revert cargo changes

* Revert cargo changes

* Comments

* Comments on test

* Update comments

* Update comments

* Add space that was removed in an earlier commit

* Cleanup

* Delete threads from the instance table

* Revert cargo file

* Remove dead code

* Clippy changes

* Clippy

* Revert unnecessary changes

* Revert unnecessary changes

* Revert unnecessary changes

* Revert unnecessary changes

* Revert unnecessary changes

* Review comments

* Review feedback

* Make thread IDs per-component-instance

* Fix config

* Tighten up completion

* Clippy

* Trigger full PR test

* Move funcref table reading

* Remove unused import

* Formatting

* Rename RemoveOnDrop

* Review feedback

* Review feedback

* Move start thread closure

* Review feedback

* Review feedback

* Correct feature for import

* Review feedback

* Disable failing tests

* Enable fixed tests

* Review feedback

* Readd tests

* Ignore task deletion test with Miri

show more ...

Implement unsafe intrinsics for compile-time builtins (#11825)

* Implement unsafe intrinsics for compile-time builtins

This commit adds the extremely unsafe
`wasmtime::CodeBuilder::expose_unsafe_in

Implement unsafe intrinsics for compile-time builtins (#11825)

* Implement unsafe intrinsics for compile-time builtins

This commit adds the extremely unsafe
`wasmtime::CodeBuilder::expose_unsafe_intrinsics` method. When enabled, the Wasm
being compiled is given access to special imports that correspond to direct,
unchecked and unsandboxed, native load and store operations. These intrinsics
are intended to be used for implementing fast, inline-able versions of WASI
interfaces that are special-cased to a particular host embedding, for example.

Compile-time builtins, as originally described in [the
RFC](https://github.com/bytecodealliance/rfcs/pull/43), are basically made up of
three parts:

1. A function inliner
2. Unsafe intrinsics
3. Component composition to encapsulate the usage of unsafe intrinsics in a safe
interface

Part (1) has been implemented in Wasmtime and Cranelift for a little while
now (see `wasmtime::Config::compiler_inlining`). This commit is part (2). After
this commit lands, part (3) can be done with `wac` and `wasm-compose`, although
follow up work is required to make the developer experience nicer and more
integrated into Wasmtime so that the APIs can look like those proposed in the
RFC.

* fill out some more docs

* fix non component model builds

* start filling out the doc example

* Factor abi params/returns out; truncate/extend pointers

* Compile unsafe intrinsics on winch as well

* prtest:full

* have the macro define the signature

* ignore tests in MIRI because MIRI can't compile Wasm

* juggle pointer provenance in `Store::data[_mut]`

* add a test for store data provenance and also fix it

* use `VmPtr` for the store data pointer

* finish writing unsafe intrinsics example

* fix up docs and rules around only accessing data from `T` in a `Store<T>`

* Only reserve space for the intrinsics' `VMFuncRef`s if they are in use

* use dangling pointers instead of options

* Rename `StoreInner::data` to `data_no_provenance` and fix some accesses to use the method accessors

* Add comments about the provenance juggling inside `StoreInner::data[_mut]`

* only compile intrinsics that are used

Turns out we don't need to add phases, we already have the info available to do
this.

* fix duplicate symbol names

show more ...

Fix some panics compiling components and resources (#11798)

In working on bytecodealliance/wasm-tools#2335 I found that there's a
few test cases in wasm-tools which Wasmtime was panicking to compile

Fix some panics compiling components and resources (#11798)

In working on bytecodealliance/wasm-tools#2335 I found that there's a
few test cases in wasm-tools which Wasmtime was panicking to compile.
The issues were all related to resource types and how information wasn't
registered ahead of time before it was translated from wasmparser's
representation to Wasmtime's representation. The high-level cause for
this had to do with how component and instance types are handled, as
opposed to concrete components or instances themselves. This was
effectively a hole in Wasmtime's translation process for components
which has never been filled out since the original implementation of
resources. The reason that this never came up before is:

* Most components don't currently import or export a component itself.
* Most components don't currently import or export component or instance
types (as opposed to values).

One of these was required to trigger this issue. The solution
implemented in this commit is to plumb the concept of an "abstract
resource" which is part of a type but not actually ever used at runtime
except for type equality during type reflection. This is expected to
have little-to-no impact on real-world components given that these
situations are rarely occurring.

show more ...

move `ConcurrentState` from `ComponentInstance` to `Store` (#11796)

* move `ConcurrentState` from `ComponentInstance` to `Store`

This has a few benefits:

- No need to specify an instance when crea

move `ConcurrentState` from `ComponentInstance` to `Store` (#11796)

* move `ConcurrentState` from `ComponentInstance` to `Store`

This has a few benefits:

- No need to specify an instance when creating or piping from a stream or future.
- No need to track the instance in an `Accessor`.
- You may now execute tasks for multiple instances in a single event loop.

The main drawback is that, if one of several instances within a single store
traps, it effectively means all instances have trapped, and the store can't be
used to create new instances. The way to avoid that is to use separate stores
for instances which must be isolated from others.

As a result of this change, a lot of code had to move from e.g. `impl Instance`
to e.g. `impl StoreOpaque`, so the diff is pretty huge, but the changes
themselves are almost entirely non-functional.

Fixes #11226

Signed-off-by: Joel Dice <[email protected]>

* fix non-component-model-async build

Signed-off-by: Joel Dice <[email protected]>

* fix outdated doc comment

Signed-off-by: Joel Dice <[email protected]>

* address review feedback

- restore `ComponentStoreData` encapsulation
- avoid conditional code duplication in `LiftContext::new`

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>

show more ...

disallow exiting a component instance during a post-return call (#11688)

* disallow exiting a component instance during a post-return call

This is a relatively recent change to the spec.

In order

disallow exiting a component instance during a post-return call (#11688)

* disallow exiting a component instance during a post-return call

This is a relatively recent change to the spec.

In order to check the `may_leave` flag in all the relevant intrinsics, I had to
plumb the caller `RuntimeComponentInstanceIndex` through a bunch of trampolines
that didn't previously need it, hence the large diff.

Note that I've added a slightly tweaked version of `trap-in-post-return.wast`
and left the upstream version disabled in `test-util/src/wast.rs` due to #11683.

Fixes #11676.

Signed-off-by: Joel Dice <[email protected]>

* fix test regressions

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>

show more ...

Refine raw `VMContext` helpers (#11670)

This redefines `Instance::from_vmctx` as a function from
`NonNull<VMContext>` to `NonNull<Instance>` to canonicalize the single
location that pointer arithmet

Refine raw `VMContext` helpers (#11670)

This redefines `Instance::from_vmctx` as a function from
`NonNull<VMContext>` to `NonNull<Instance>` to canonicalize the single
location that pointer arithmetic is done but otherwise require callers
to handle the safety requirements of safe references and such.

show more ...

Require a store in `catch_unwind_and_record_trap` (#11441)

* Require a store in `catch_unwind_and_record_trap`

This commit does some preparatory refactoring for #11326 to ensure that
a store is ava

Require a store in `catch_unwind_and_record_trap` (#11441)

* Require a store in `catch_unwind_and_record_trap`

This commit does some preparatory refactoring for #11326 to ensure that
a store is available when trap information is being processed. Currently
this doesn't leverage the new parameter but it should be leverage-able
in #11326.

* Review comments

show more ...

use a single table per instance for resources, waitables, etc. (#11374)

* use a single table per instance for resources, waitables, etc.

Per https://github.com/WebAssembly/component-model/pull/513,

use a single table per instance for resources, waitables, etc. (#11374)

* use a single table per instance for resources, waitables, etc.

Per https://github.com/WebAssembly/component-model/pull/513, the spec now puts
resources, waitables, waitable sets, subtasks, and error contexts in the same
table per instance. This updates the implementation to match.

- Combine the `ResourceTable` and `StateTable` data structures into a single `HandleTable` structure

- Rename `ComponentInstance::instance_resource_tables` to `instance_handle_tables`

- Remove `ConcurrentState::waitable_tables` and `error_context_tables` in favor of the above

- Move various associated functions from `ConcurrentState` to `ComponentInstance` so they can access `instance_resource_tables`

While I was doing table-related things, I also updated `concurrent::Table::new`
to reserve the zero handle to mean "invalid". This won't affect what the guest
sees in any way, but it allows us to use `TableId::new(0)` to invalidate
host-owned handles in e.g. `{Stream,Future}{Reader,Writer}::close`.

Fixes #11189

Signed-off-by: Joel Dice <[email protected]>

Re-internalize `ResourceKind` to `mod handle_table`

Remove `ResourceKind`

Start flattening the representation of `Slot`

Internalize `get_mut_handle_by_index`

Internalize implementation details such as the representation of slots
to and make methods a bit more targeted in their functionality.

Internalize more details of `HandleTable`

Don't expose `HandleKind` and of per-function methods for operating on
the various kinds of handles that reside in the table.

Flatten the representation of `Slot`

There's still some more work to do for host/guest resource handles, but
this helps realize the goal of the previous refactorings in the
meantime.

* stop using `HandleTable::reps_to_indexes` when delivering events

Per review feedback, we'd like to get rid of `HandleTable::reps_to_indexes`
entirely. This commit doesn't go quite that far, but now we only use it for
`error-context` handles. For waitables, which can only be referenced by at most
one guest at a time, we now store the guest handle in `WaitableCommon::handle`
and retrieve it from there when delivering an event for that waitable.

For `error-context` handles, the spec requirement that we always lower the same
handle for the same `error-context`, combined with the fact that an
`error-context` may be referenced by more than one component instance at a time,
means we still need some general way to convert a host rep plus component index
into a handle. Going forward, we could consider either removing that "same
handle" requirement from the spec or consider an alternative implementation
(e.g. storing a `HashMap<RuntimeComponentIndex, usize>` in the `ErrorContext`
host state for keeping track of the handles for each referencing instance.

Signed-off-by: Joel Dice <[email protected]>

* remove `HandleTable::reps_to_indexes`

Turns out the spec no longer requires that guests receive the same handle for a
given `error-context` as the one they already have, so we no longer need this
field -- nor do we need to maintain a per-component-instance reference count.

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>

show more ...

Package component model options in an index (#11324)

* Package component model options in an index

This commit is a refactoring of how canonical ABI options are handled
during compilation and runti

Package component model options in an index (#11324)

* Package component model options in an index

This commit is a refactoring of how canonical ABI options are handled
during compilation and runtime. Previously options were "exploded"
meaning that options were all passed around as parameters but this was a
bummer for a few reasons:

* This is `unsafe`-prone as options have raw pointers such as memory and
functions. This meant that all functions/operations working with
options were fundamentally `unsafe`.

* This was unwieldy as the set of options continues to grow larger over
time. The `VMLoweringCallee` function previously had 10+ parameters,
most of which were canonical ABI options.

To solve these two problems options are now intern'd at compile time to
an `OptionsIndex`, a 32-bit value. This is stored as a new table in
component metadata and consulted at runtime. This means that
`OptionsIndex` can be passed around with an instance for a much safer
means of threading options around. Additionally there's no need to pass
around all parameters individually and instead just one parameter,
`OptionsIndex`, need be threaded through.

This commit additionally overhauls trampoline generation for the
component compiler to avoid a function-per-intrinsic and instead have a
single function that all intrinsics use. I found this easier to
understand and helps codify that all intrinsics are basically the same
with some minor details differing between them.

The end result of this is (hopefully) a net simplification of the
component compiler in addition to a large amount of removal of `unsafe`
code in the async implementation as only safe types are passed around
now instead of raw pointers.

Closes #10143
Closes #11188

* Fill out some TODO comments

show more ...

Deny `unsafe_op_in_unsafe_fn` in `wasmtime::runtime::vm` (#11312)

* Deny `unsafe_op_in_unsafe_fn` in `wasmtime::runtime::vm`

Slowly expanding this lint to more of the crate.

prtest:full

* Fix lin

Deny `unsafe_op_in_unsafe_fn` in `wasmtime::runtime::vm` (#11312)

* Deny `unsafe_op_in_unsafe_fn` in `wasmtime::runtime::vm`

Slowly expanding this lint to more of the crate.

prtest:full

* Fix lints in custom module

* Fix some lints with miri

* Fix non-VM build

* Fix arm windows

show more ...

Remove `Arc::clone` when creating `LiftContext` (#11253)

A small amount of `unsafe` code is required to enable this but the
recent refactorings to use `Pin<&mut ComponentInstance>` everywhere
makes

Remove `Arc::clone` when creating `LiftContext` (#11253)

A small amount of `unsafe` code is required to enable this but the
recent refactorings to use `Pin<&mut ComponentInstance>` everywhere
makes this a much easier piece of `unsafe` code to verify.

show more ...

Gut `vm::Export` to mostly be `crate::Extern` (#11229)

* Remove `Table::from_wasmtime_table`

This commit removes the unsafe function `Table::from_wasmtime_table`.
This goes a bit further and remove

Gut `vm::Export` to mostly be `crate::Extern` (#11229)

* Remove `Table::from_wasmtime_table`

This commit removes the unsafe function `Table::from_wasmtime_table`.
This goes a bit further and removes `ExportTable` entirely as well which
means that table lookup on a `vm::Instance` directly returns a
`wasmtime::Table` without any need to translate back-and-forth.

* Remove `Tag::from_wasmtime_tag`

Like the previous commit, but for tags.

* Remove `Global::from_wasmtime_global`

Like the previous commit, but for globals.

* Remove `Memory::from_wasmtime_memory`

Like the previous commit, but for memories.

* Remove `Func::from_wasmtime_function`

Like previous commits, but for functions.

* Fix lints

* Fill out missing safety comment

* Review comments

show more ...

implement Component Model async ABI (#11127)

* implement Component Model async ABI

This commit replaces the stub functions and types in
`wasmtime::runtime::component::concurrent` and its submodules

implement Component Model async ABI (#11127)

* implement Component Model async ABI

This commit replaces the stub functions and types in
`wasmtime::runtime::component::concurrent` and its submodules with the working
implementation developed in the `wasip3-prototyping` repo. For ease of review,
it does not include any new tests; I'll add those in a follow-up commit.

Note that this builds on #11123; only the most recent commit is new.

Signed-off-by: Joel Dice <[email protected]>

* clear params pointer in `call_async` when future is dropped

This ensures that the closure we pass to `prepare_call` will never see a stale
pointer.

Note that this could potentially be made more efficient; I'm starting with a
simple solution, and we can refine from there.

Signed-off-by: Joel Dice <[email protected]>

* Remove unsafety from accessing concurrent async state

* Remove a dead variant when async is disabled

* Add tests for `tls.rs` unsafe code

* Refactor `AbortHandle`

* Don't close over the entire future in the `AbortHandle`, instead
change it to just the bare minimum state to manage aborts.
* Move aborting logic into a helper `AbortHandle::run` function which
handles the is-this-aborted-check internally.
* Refactor some logic around spawns how `AbortHandle` is
managed/created.

* Internalize some functions/types

* add FIXME comment to `states.rs`

Signed-off-by: Joel Dice <[email protected]>

* reference issue 11190 in `table.rs` TODO

Signed-off-by: Joel Dice <[email protected]>

* switch `use` directives to conventional syntax

Signed-off-by: Joel Dice <[email protected]>

* remove redundant accessor methods

Signed-off-by: Joel Dice <[email protected]>

* reference issue 11191 in `yield` TODO comments

Signed-off-by: Joel Dice <[email protected]>

* replace `dummy_waker` with `Waker::noop`

Signed-off-by: Joel Dice <[email protected]>

* remove obsolete `AsyncState::spawned_tasks` field

Signed-off-by: Joel Dice <[email protected]>

* only call post-return automatically for `call_concurrent`

This restores the original behavior of requiring explicit post-return calls for
`call[_async]` invocations.

Signed-off-by: Joel Dice <[email protected]>

* Favor function arguments before closures

* Simplify `watch` a bit

Mostly move unnecessary state out of the `Arc`.

* fix task handle leaks and add test coverage

We weren't always disposing of guest or host task handles once they became
unreachable. This adds a couple of hidden methods which integration tests may
use to guard against use-after-delete, double-delete, and leak bugs regarding
waitable handles. It also tightens up handle management in `concurrent.rs` to
ensure those tests pass.

Signed-off-by: Joel Dice <[email protected]>

* Encapsulate type erasure in stream buffers

Don't rely on all buffers to handle `TypeId` and assertions and such,
instead have a helper type which is the one location of the assertions
and everything else can stay typed.

* Remove some methods ending in underscores

* Refactor unsafety in `buffers.rs`

Mostly move away from raw pointers and instead use utilities like
`&[MaybeUninit<T>]`. Also make `WriteBuffer` an `unsafe` trait after
absorbing the `TakeBuffer` trait. Update all safety-related comments
here and there too.

* remove task on drop in `TypedFunc::call_async`

This avoids the need for an `Arc`.

Signed-off-by: Joel Dice <[email protected]>

* remove obsolete clause from `FutureReader::read` docs

Signed-off-by: Joel Dice <[email protected]>

* unhide and expand docs for `WriteBuffer` and `ReadBuffer`

Signed-off-by: Joel Dice <[email protected]>

* add optional `component-model-async-bytes` feature

This gates interop with the `bytes` crate, making it optional and non-default.

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>
Co-authored-by: Alex Crichton <[email protected]>

show more ...