Fix overlap assertions in string transcoding (#12893)

Fixes an off-by-one assertion which is possible to happen when linear
memories are directly adjacent to each other. While an esoteric
situation,

Fix overlap assertions in string transcoding (#12893)

Fixes an off-by-one assertion which is possible to happen when linear
memories are directly adjacent to each other. While an esoteric
situation, it's technically possible as the added test shows.

Closes #12678

show more ...

Relax panics in async/futures to traps/errors (#12688)

* Relax panics in async/futures to traps/errors

This commit is an admittance that I don't believe we're going to get
to a point where we are c

Relax panics in async/futures to traps/errors (#12688)

* Relax panics in async/futures to traps/errors

This commit is an admittance that I don't believe we're going to get
to a point where we are confident enough in the fuzzing of
component-model-async such that we could confidently say we're
exercising the vast majority of possible panics. Development of
component-model-async has shown a steady trickle of panics over the
course of the development of the feature, and this trend has been
persistent over time as well.

An attempt was made in #12119 to add a fuzzer dedicated to async events
but that didn't actually find anything in development and it has missed
a number of panics present before and discovered after its introduction.
Overall I do not know how to improve the fuzzer to the point that it
would find pretty much all of the existing async-related panics over
time.

To help address this concern of the `concurrent.rs` implementation this
commit goes through and replaces things like `unwrap()`, `assert!`,
`panic!`, and `unreachable!` with an error-producing form. The benefit
of this is that a bug in the implementation is less likely to result in
a panic and instead just results in a non-spec-compliant trap. The
downside of doing this though is that it can become unclear what errors
are "first class traps", or expected to be guest reachable, and which
are expected to be bugs in Wasmtime. To help address this I've performed
a few refactorings here as well.

* Some traps previously present as error strings are now promoted to
using `Trap::Foo` instead. This has some refactoring of the Rust/C
side as well to make it easier to define new variants. Tests were
additionally added for any trap messages that weren't previously
tested as being reachable.

* A new `bail_bug!` macro was added (internally) for Wasmtime. This is
coupled with a concrete `WasmtimeBug` error type (exported as
`wasmtime::WasmtimeBug`). The intention is that `bail!` continues to
be "here's a string and I'm a bit too lazy to make a concrete error"
while `bail_bug!` indicates "this is a bug in wasmtime please report
this if you see it".

The rough vision is that if an error condition is reached, and the system
is not broken in such a way that panicking is required, then `bail_bug!`
can be used to indicate a bug in Wasmtime as opposed to panicking. This
reduces the real-world impact of hitting these scenarios by downgrading a
CVE-worthy `panic!` into a bug-worthy non-spec-compliant trap. Not all
panics are able to be transitioned to this as some are load bearing from
a safety perspective or similar (or indicate something equally broken),
but the vast majority of cases are suitable for "return a trap, lock
down the store, and let destructors take care of everything else".

This change additionally has resulted in API changes for `FutureReader`
and `StreamReader`. For example creation of these types now returns a
`Result` for when the `ResourceTable` is full, for example, instead of
panicking.

* Fix CI build

* Translate `WasmtimeBug` to panics in debug mode

* Review comments

* Refactor some stream methods for fewer panics

show more ...

Update to wasm-tools 245 (#12571)

* Intrinsics updates

* Remove ds_store

* Update to wasm-tools 245

* Add vets

* Fixup tests

* Add missing feature for `indexmap`

---------

Co-authored-by: Sy

Update to wasm-tools 245 (#12571)

* Intrinsics updates

* Remove ds_store

* Update to wasm-tools 245

* Add vets

* Fixup tests

* Add missing feature for `indexmap`

---------

Co-authored-by: Sy Brand <[email protected]>

show more ...

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 ...

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 ...

consistently create thread and task when entering component instance (#12379)

* consistently create thread and task when entering component instance

Previously, we weren't creating a new thread or

consistently create thread and task when entering component instance (#12379)

* consistently create thread and task when entering component instance

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

* fix warnings when component-model-async feature disabled

* address review feedback

* push a task when calling a resource dtor host-to-guest

* add tests which call `thread.index` from realloc and post-return functions

...and assert that the indexes match as expected.

Getting the post-return test to pass required moving the call to
`StoreOpaque::exit_sync_call` to after the post-return function is called.

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 ...

Do not re-export `anyhow!` in the `wasmtime::prelude` (#12298)

We are trying to move to `format_err!` instead.

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 ...

generate precise traps in component adapters (#12215)

* generate precise traps in component adapters

Previously, we generated a generic `unreachable` instruction for each trap in a
fused adapter, p

generate precise traps in component adapters (#12215)

* generate precise traps in component adapters

Previously, we generated a generic `unreachable` instruction for each trap in a
fused adapter, plus some metadata to recover the specific kind of trap.
However, that metadata was never hooked up to anything, so we only got a generic
`unreachable` message at runtime.

This commit removes the metadata tracking and instead simply calls a host
intrinsic, passing the trap code as a parameter. This is somewhate pessimal
compared with what we had before, but improves ergonomics and allows us to avoid
forking as many tests from the component-model repo.

If performance becomes an issue, we can easily add an option to skip the host
call and only emit an `unreachable` instruction as before.

Note that I've removed forked versions of three tests in favor of their upstream
equivalents.

Fixes #11683

* update `strings.rs` tests

* bless disas tests

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 ...

Remove support for `backpressure.set` (#12130)

This canonical intrinsic which is part of component-model-async support
is supplanted by `backpressure.{inc,dec}`. This wasn't removed when they
were a

Remove support for `backpressure.set` (#12130)

This canonical intrinsic which is part of component-model-async support
is supplanted by `backpressure.{inc,dec}`. This wasn't removed when they
were added to keep existing programs working, but that's no longer
necessary.

show more ...

trap on blocking call in sync task before return (#12043)

* trap on blocking call in sync task before return

This implements a spec change (PR pending) such that tasks created for calls to
synchron

trap on blocking call in sync task before return (#12043)

* trap on blocking call in sync task before return

This implements a spec change (PR pending) such that tasks created for calls to
synchronous exports may not call potentially-blocking imports or return `wait`
or `poll` callback codes prior to returning a value. Specifically, the
following are prohibited in that scenario:

- returning callback-code.{wait,poll}
- sync calling an async import
- sync calling subtask.cancel
- sync calling {stream,future}.{read,write}
- sync calling {stream,future}.cancel-{read,write}
- calling waitable-set.{wait,poll}
- calling thread.suspend

This breaks a number of tests, which will be addressed in follow-up commits:

- The `{tcp,udp}-socket.bind` implementation in `wasmtime-wasi` is implemented
using `Linker::func_wrap_concurrent` and thus assumed to be async, whereas the
WIT interface says they're sync, leading to a type mismatch error at runtime.
Alex and I have discussed this and have a general plan to address it.

- A number of tests in the tests/component-model submodule that points to the
spec repo are failing. Those will presumably be fixed as part of the upcoming
spec PR (although some could be due to bugs in this implementation, in which
case I'll fix them).

- A number of tests in tests/misc_testsuite are failing. I'll address those in
a follow-up commit.

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

* call `check_may_leave` before `check_blocking`

`check_blocking` needs access to the current task, but that's not set for
post-return functions since those should not be calling _any_ imports at all, so
first check for that.

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

* fix `misc_testsuite` test regressions

This amounts to adding `async` to any exported component functions that might
need to block.

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

* simplify code in `ConcurrentState::check_blocking`

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

* make `thread.yield` a no-op in non-blocking contexts

Per the proposed spec changes, `thread.yield` should return control to the guest
immediately without allowing any other thread to run. Similarly, when an
async-lifted export or callback returns `CALLBACK_CODE_YIELD`, we should call
the callback again immediately without allowing another thread to run.

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

* fix build when `component-model-async` feature disabled

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

* fix more test regressions

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

* fix more test regressions

Note that this temporarily updates the `tests/component-model` submodule to the
branch for https://github.com/WebAssembly/component-model/pull/577 until that PR
is merged.

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

* tweak `Trap::CannotBlockSyncTask` message

This clarifies that such a task cannot block prior to returning.

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

* fix cancel_host_future test

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

* trap sync-lowered, guest->guest async calls in sync tasks

I somehow forgot to address this earlier. Thanks to Luke for catching this.

Note that this commit doesn't include test coverage, but Luke's forthecoming
tests in the `component-model` repo will cover it, and we'll pull that in with
the next submodule update.

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

* switch back to `main` branch of `component-model` repo

...and skip or `should_fail` the tests that won't pass until
https://github.com/WebAssembly/component-model/pull/578 is merged.

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

* add `trap-if-block-and-sync.wast`

We'll remove this again in favor of the upstream version once
https://github.com/WebAssembly/component-model/pull/578 has been merged.

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

* address review feedback

- Assert that `StoreOpaque::suspend` is not called in a non-blocking context except in specific circumstances

- Typecheck async-ness for dynamic host functions

- Use type parameter instead of value parameter in `call_host[_dynamic]`

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

* add explanation comments to `check_blocking` calls

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

* fix fuzz test oracle for async functions

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

---------

Signed-off-by: Joel Dice <[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 ...

Update nightly rust used in CI (#11856)

* Update nightly rust used in CI

A few more warnings are cropping up so squash them.

* Frob the `unreachable_code` lint

Looks like nightly rust has gotten

Update nightly rust used in CI (#11856)

* Update nightly rust used in CI

A few more warnings are cropping up so squash them.

* Frob the `unreachable_code` lint

Looks like nightly rust has gotten much more aggressive about linting on
unreachable code.

prtest:full

* Allow some more warnings...

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 ...

Implement `backpressure.{inc,dec}` (#11661)

Added to the async specification in WebAssembly/component-model#560
these are minor adaptations to the preexisting `backpressure.set`
intrinsic and are in

Implement `backpressure.{inc,dec}` (#11661)

Added to the async specification in WebAssembly/component-model#560
these are minor adaptations to the preexisting `backpressure.set`
intrinsic and are intended to replace it. The `backpressure.set`
intrinsic will remain until tooling propagates to understand
`backpressure.{inc,dec}`.

show more ...

Update to wasm-tools 239 (#11660)

Changes include:

* `async` option on some CM intrinsics renamed to `cancellable`
* New `backpressure.{inc,dec}` intrinsics
* New component-model-threading-related

Update to wasm-tools 239 (#11660)

Changes include:

* `async` option on some CM intrinsics renamed to `cancellable`
* New `backpressure.{inc,dec}` intrinsics
* New component-model-threading-related intrinsics

New features aren't yet implemented, they're left for future PRs.

show more ...

support non-async `{stream,future}.cancel-{read,write}` (#11625)

* support non-async `{stream,future}.cancel-{read,write}`

During my earlier stream API refactoring, I had forgotten to support or te

support non-async `{stream,future}.cancel-{read,write}` (#11625)

* support non-async `{stream,future}.cancel-{read,write}`

During my earlier stream API refactoring, I had forgotten to support or test
synchronous cancellation; this commit does both. In the process, I realized the
future API ought to be updated to support blocking cancellation just like the
stream API, so I made that change as well.

This also adds `{Source,Destination}::reborrow` functions, allowing instances of
those types to be reborrowed, such that they may be passed as parameters but
also used again.

Note that I had to move some functions from `impl ConcurrentState` to `impl
Instance` in order to access the store and suspend the current fiber when
synchronously cancelling.

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

* reduce code duplication

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

---------

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

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 ...

refactor `{Stream,Future}|{Reader,Writer}` APIs and internals (#11325)

* refactor `{Stream,Future}|{Reader,Writer}` APIs and internals

This makes a several changes to how `{Stream,Future}|{Reader,W

refactor `{Stream,Future}|{Reader,Writer}` APIs and internals (#11325)

* refactor `{Stream,Future}|{Reader,Writer}` APIs and internals

This makes a several changes to how `{Stream,Future}|{Reader,Writer}` work to
make them more efficient and, in some ways, more ergonomic:

- The background tasks have been removed, allowing reads and writes to complete without task context switching. We now only allocate and use oneshot channels lazily when the other end is not yet ready; this improves real world performance benchmarks (e.g. wasi-http request handling) considerably.

- Instances of `{Stream,Future}Reader` can now be lifted and lowered directly; no need for `Host{Stream,Future}` anymore.

- The type parameter for `Stream{Reader,Writer}` no longer refers to the buffer type -- just the payload type (i.e. `StreamReader<u8>` instead of `StreamReader<Vec<u8>>`), meaning any buffer type may be used for a given read or write operation. This also means the compiler needs help with type inference less often when calling `Instance::stream`.

- Instances of `{Stream,Future}|{Reader,Writer}` now require access to the store in order to be disposed of properly. I've added RAII wrapper structs (`WithAccessor[AndValue]`) to help with this, and also updated `Store::drop` and `Instance::run_concurrent` to ensure the store thread-local is set when dropping futures closing over `&Accessor`s.

- In order to ensure that resources containing `{Stream,Future}|{Reader,Writer}` instances are disposed of properly, I've added `LinkerInstance::resource_concurrent` and have updated `wasmtime-wit-bindgen` to use it. This gives resource drop functions access to a `StoreContextMut` via an `Accessor`, allowing the stream and future handles to be disposed of.
- In order to make this work, I had to change `Accessor::instance` from a `Instance` to an `Option<Instance>`, which is awkward but temporary since we're planning to remove `Accessor::instance` entirely once we've moved concurrent state from `ComponentInstance` to `Store`.

That problem of disposal is definitely the most awkward part of all this. In
simple cases, it's easy enough to ensure that read and write handles are
disposed of properly, but both `wasmtime-wasi` and `wasmtime-wasi-http` have
some pretty complicated functions where handles are passed between tasks and/or
stored inside resources, so it can be tricky to ensure proper disposal on all
code paths. I'm open to ideas for improving this, but I suspect we'll need new
Rust language features (e.g. linear types) to make it truly ergonomic, robust,
and efficient.

While testing the above, I discovered an issue with `Instance::poll_until` such
that it would prematurely give up and return a "deadlock" trap error, believing
that there was no further work to do, even though the future passed to it was
ready to resolve the next time it was polled. I've fixed this by polling it one
last time and only trapping if it returns pending.

Note that I've moved a few associated functions from `ConcurrentState` to
`Instance` (e.g. `guest_drop_writable` and others) since they now need access to
the store; they're unchanged otherwise. Apologies for the diff noise.

Finally, I've tweaked how `wasmtime serve` to poll the guest for content before
handing the response to Hyper, which helps performance by ensuring the first
content chunk can be sent with the same TCP packet as the beginning of the
response.

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

fix wasi p3 build and test failures

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

use `ManuallyDrop` instead of `Option` in `Dropper`

This allows us to drop its `value` field in-place, i.e. without moving it,
thereby upholding the `Pin` guarantee.

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

address review comments

- Remove `DropWithStoreAndValue` and friends; go back to taking a `fn() -> T` parameter in `Instance::future` instead
- Make `DropWithStore::drop[_with]` take `&mut self` instead of `self`
- Make `WithAccessor` and `DropWithStore` private
- Instead, I've added public `Guarded{Stream,Future}{Reader,Writer}` types for RAII
- and also `{Stream,Future}{Reader,Writer}::close[_with]` methods
- Use RAII in `FutureReader::read` and `FutureWriter::write` to ensure handles are dropped if the `Future` is dropped

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

* lower host stream/future writes in background task

This avoids unsoundness due to guest realloc calls while there are host embedder
frames on the stack.

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

* fix `tcp.rs` regressions

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 ...