Skip to content

Commit 421136d

Browse files
dicejalexcrichton
andauthored
generalize async fiber abstraction (#11114)
* generalize async fiber abstraction As part of the work implementing the new Component Model async ABI in the `wasip3-prototyping` repo, I've generalized the `FiberFuture` abstraction in `wasmtime::runtime::store::async_` to support fibers which can either retain exclusive access to the store across suspend points or release it. The latter allows the store to be used by the `component-model-async` event loop and/or other fibers to run before the original fiber resumes, which is the key to allowing multiple fibers to run concurrently, passing control of the store back and forth. In the case of Pulley, the above generalization means we also need to give each fiber its own `Interpreter` so that multiple concurrent fibers don't clobber each other's state. Concretely, this moves a lot of the code out of `async_.rs` and into a new `fiber.rs` submodule which will be shared with the `component-model-async` implementation. This also pulls in a new `StoreToken<T>` utility which has been useful in `wasip3-prototyping` to safely convert from a `&mut dyn VMStore` to a `StoreContextMut<'a, T>` when we previously witnessed a conversion in the other direction. Note that I've added a `'static` bound to the `VMStore` trait, which simplifies use of `&mut dyn VMStore`, avoiding thorny lifetime issues. Signed-off-by: Joel Dice <[email protected]> * address review feedback Signed-off-by: Joel Dice <[email protected]> * fix miri-flagged stacked borrow violation As part of my earlier effort to unify the fiber abstractions in the `wasmtime` crate, I changed a `*mut StoreFiber` field to a `&mut StoreFiber`, not realizing that it resulted in a mutable alias at runtime and thus undefined behavior. Miri caught it, fortunately. Signed-off-by: Joel Dice <[email protected]> * remove unneeded `Send` bounds Signed-off-by: Joel Dice <[email protected]> * address more review feedback Main changes: - Make `resume_fiber[_raw]` take a `&mut StoreOpaque` parameter to make its unsafe internals easier to reason about, safety-wise. - Panic if `StoreFiber::drop` is called on an in-progress fiber without having called `StoreFiber::dispose` to gracefully end it first. - (Re)introduce `FiberFuture`, which closes over a `&mut StoreOpaque` and uses it to call `StoreFiber::dispose` on drop. This will require a few more changes to make it usable by `concurrent.rs`, but I'll save those changes for a later PR. Signed-off-by: Joel Dice <[email protected]> * address more review feedback Signed-off-by: Joel Dice <[email protected]> * update `impl Send For StoreFiber` comment Signed-off-by: Joel Dice <[email protected]> * Remove currently-extraneous `Result<()>` from fibers May be needed for concurrent bits, but for now not necessary. * Use safe pointers instead of raw pointers It's predicted Miri won't like this, but for now in-repo it's ok with it. * Fold more responsibility into `resume_fiber_raw` Remove the need for the function entirely and replace it with `resume_fiber`. * Remove channels from async fibers Can use stack-based closures/results to transmit the result instead of needing a channel. * Fold `on_fiber_raw` directly into `on_fiber` The `on_fiber` function is small enough it should be possible to do so. * Don't use `Option` in `FiberFuture` Leave the fiber non-optional at-rest so it's always available for the destructor. * Fold `suspend` functions together Small shims, not otherwise public at this time, so remove a layer of indirection. * Move stack limit management to `FiberResumeState` Helps remove some raw pointers that are held for a long time within `AsyncCx` * add some doc comments to `fiber.rs` Signed-off-by: Joel Dice <[email protected]> * update `fiber.rs` and friends to match CM async requirements This adds a `resolve_or_release` function, which `Instance::resume_fiber` will use when current `concurrent.rs` stub is replaced by a real implementation. Signed-off-by: Joel Dice <[email protected]> * fix non-component-model-async build warnings Signed-off-by: Joel Dice <[email protected]> * make `resume_fiber` private in `fiber.rs` Signed-off-by: Joel Dice <[email protected]> * Shrink `PollContext` state Move management of the async guard range elsewhere to the normal save/restore area. * Refactor `AsyncCx`, reduce `unsafe` * Remove the `AsyncCx` type from Wasmtime as it's inherently `unsafe` to use, instead bundle operations directly on a `Store*` reference. * Don't retain pointers-to-pointers within the roughly-equivalent `BlockingContext` created in this PR. Instead when a blocking context is created "take" the metadata from the store to assert exclusive ownership of the pointers. * Refactor how `&mut Context<'_>` is passed around, namely thread it through fiber parameters to model resumption as registering a new context to poll with. * Remove `PollContext` in favor of directly storing a pointer as it's now mostly an empty structure. * Minor refactorings to make things more future-refactorable and/or clear in a few places. * Refactor management of the "current suspend" and "current future context" pointers. These are now null'd out on resumption and asserted null on suspension. * Remove the need for a generic `Reset` structure in the fiber bits as it's a pretty dangerous structure to have in general. The end result of this refactoring is that all usage of `block_on` is now safe and additionally many of the internals of the implementation are safer than they were before * Adjust some lint attributes * Make manipulation of `AsyncState` safe No need for raw pointers with recent refactorings. * Fix dead code warning * More dead code warnings * Cut down on raw pointers in fiber.rs * Move executor save/restore to normal fiber state save/restore * Bikeshed a method name * update comment in make_fiber Signed-off-by: Joel Dice <[email protected]> * fix machports build Signed-off-by: Joel Dice <[email protected]> --------- Signed-off-by: Joel Dice <[email protected]> Co-authored-by: Alex Crichton <[email protected]>
1 parent a6d1244 commit 421136d

File tree

19 files changed

+1211
-696
lines changed

19 files changed

+1211
-696
lines changed

benches/call.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ fn bench_host_to_wasm<Params, Results>(
134134
typed_params: Params,
135135
typed_results: Results,
136136
) where
137-
Params: WasmParams + ToVals + Copy,
138-
Results: WasmResults + ToVals + Copy + PartialEq + Debug,
137+
Params: WasmParams + ToVals + Copy + Sync,
138+
Results: WasmResults + ToVals + Copy + Sync + PartialEq + Debug + 'static,
139139
{
140140
// Benchmark the "typed" version, which should be faster than the versions
141141
// below.
@@ -628,7 +628,8 @@ mod component {
628628
+ PartialEq
629629
+ Debug
630630
+ Send
631-
+ Sync,
631+
+ Sync
632+
+ 'static,
632633
{
633634
// Benchmark the "typed" version.
634635
c.bench_function(&format!("component - host-to-wasm - typed - {name}"), |b| {

crates/wasmtime/src/runtime.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ pub(crate) mod code_memory;
3434
#[cfg(feature = "debug-builtins")]
3535
pub(crate) mod debug;
3636
pub(crate) mod externals;
37+
#[cfg(feature = "async")]
38+
pub(crate) mod fiber;
3739
pub(crate) mod gc;
3840
pub(crate) mod instance;
3941
pub(crate) mod instantiate;

crates/wasmtime/src/runtime/component/func.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ impl Func {
547547
let mut store = store.as_context_mut();
548548
assert!(
549549
store.0.async_support(),
550-
"cannot use `call_async` without enabling async support in the config"
550+
"cannot use `post_return_async` without enabling async support in the config"
551551
);
552552
// Future optimization opportunity: conditionally use a fiber here since
553553
// some func's post_return will not need the async context (i.e. end up

crates/wasmtime/src/runtime/component/func/typed.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ where
179179
) -> Result<Return>
180180
where
181181
Params: Send + Sync,
182-
Return: Send + Sync,
182+
Return: Send + Sync + 'static,
183183
{
184184
let mut store = store.as_context_mut();
185185
assert!(

crates/wasmtime/src/runtime/component/linker.rs

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -441,10 +441,8 @@ impl<T: 'static> LinkerInstance<'_, T> {
441441
self.engine.config().async_support,
442442
"cannot use `func_wrap_async` without enabling async support in the config"
443443
);
444-
let ff = move |mut store: StoreContextMut<'_, T>, params: Params| -> Result<Return> {
445-
let async_cx = store.as_context_mut().0.async_cx().expect("async cx");
446-
let future = f(store.as_context_mut(), params);
447-
unsafe { async_cx.block_on(Pin::from(future)) }?
444+
let ff = move |store: StoreContextMut<'_, T>, params: Params| -> Result<Return> {
445+
store.block_on(|store| f(store, params).into())?
448446
};
449447
self.func_wrap(name, ff)
450448
}
@@ -603,12 +601,10 @@ impl<T: 'static> LinkerInstance<'_, T> {
603601
self.engine.config().async_support,
604602
"cannot use `func_new_async` without enabling async support in the config"
605603
);
606-
let ff = move |mut store: StoreContextMut<'_, T>, params: &[Val], results: &mut [Val]| {
607-
let async_cx = store.as_context_mut().0.async_cx().expect("async cx");
608-
let future = f(store.as_context_mut(), params, results);
609-
unsafe { async_cx.block_on(Pin::from(future)) }?
604+
let ff = move |store: StoreContextMut<'_, T>, params: &[Val], results: &mut [Val]| {
605+
store.with_blocking(|store, cx| cx.block_on(Pin::from(f(store, params, results)))?)
610606
};
611-
self.func_new(name, ff)
607+
return self.func_new(name, ff);
612608
}
613609

614610
/// Defines a [`Module`] within this instance.
@@ -676,12 +672,8 @@ impl<T: 'static> LinkerInstance<'_, T> {
676672
let dtor = Arc::new(crate::func::HostFunc::wrap_inner(
677673
&self.engine,
678674
move |mut cx: crate::Caller<'_, T>, (param,): (u32,)| {
679-
let async_cx = cx.as_context_mut().0.async_cx().expect("async cx");
680-
let future = dtor(cx.as_context_mut(), param);
681-
match unsafe { async_cx.block_on(Pin::from(future)) } {
682-
Ok(Ok(())) => Ok(()),
683-
Ok(Err(trap)) | Err(trap) => Err(trap),
684-
}
675+
cx.as_context_mut()
676+
.block_on(|store| dtor(store, param).into())?
685677
},
686678
));
687679
self.insert(name, Definition::Resource(ty, dtor))?;

crates/wasmtime/src/runtime/component/store.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ impl ComponentStoreData {
2828
pub fn next_component_instance_id(&self) -> ComponentInstanceId {
2929
self.instances.next_key()
3030
}
31+
32+
#[cfg(feature = "component-model-async")]
33+
pub(crate) fn drop_fibers(store: &mut StoreOpaque) {
34+
_ = store;
35+
// This function will actually do something when runtime support for
36+
// `component-model-async` is merged.
37+
}
3138
}
3239

3340
impl StoreData {

0 commit comments

Comments
 (0)