Skip to content

Commit 6e21cf1

Browse files
authored
Remove StoreOpaque::async_yield_impl (#11482)
* Remove `StoreOpaque::async_yield_impl` Now that we have all this fancy support for natively running `async` things this commit refactors fuel/epochs to use it. This simplifies the `VMStore` trait, removes a usage of `block_on`, and helps keep the `async` boundary close to the libcall entrypoint rather than further down the stack. This all in turn enables using rustc to check our stack-locals for non-`Send` values instead of pinky promising that we're doing the right thing everywhere. * Clean up some code movement and comments * Fix a merge conflict
1 parent 4abb213 commit 6e21cf1

File tree

5 files changed

+78
-123
lines changed

5 files changed

+78
-123
lines changed

crates/wasmtime/src/runtime/fiber.rs

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -343,35 +343,7 @@ impl<T> StoreContextMut<'_, T> {
343343
}
344344
}
345345

346-
impl<T> crate::store::StoreInner<T> {
347-
/// Blocks on the future computed by `f`.
348-
///
349-
/// # Panics
350-
///
351-
/// Panics if this is invoked outside the context of a fiber.
352-
pub(crate) fn block_on<R>(
353-
&mut self,
354-
f: impl FnOnce(StoreContextMut<'_, T>) -> Pin<Box<dyn Future<Output = R> + Send + '_>>,
355-
) -> Result<R> {
356-
BlockingContext::with(self, |store, cx| {
357-
cx.block_on(f(StoreContextMut(store)).as_mut())
358-
})
359-
}
360-
}
361-
362346
impl StoreOpaque {
363-
/// Blocks on the future computed by `f`.
364-
///
365-
/// # Panics
366-
///
367-
/// Panics if this is invoked outside the context of a fiber.
368-
pub(crate) fn block_on<R>(
369-
&mut self,
370-
f: impl FnOnce(&mut Self) -> Pin<Box<dyn Future<Output = R> + Send + '_>>,
371-
) -> Result<R> {
372-
BlockingContext::with(self, |store, cx| cx.block_on(f(store).as_mut()))
373-
}
374-
375347
/// Creates a `BlockingContext` suitable for blocking on futures or
376348
/// suspending the current fiber.
377349
///

crates/wasmtime/src/runtime/store.rs

Lines changed: 27 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ use crate::runtime::vm::{
9696
SignalHandler, StoreBox, Unwind, VMContext, VMFuncRef, VMGcRef, VMStore, VMStoreContext,
9797
};
9898
use crate::trampoline::VMHostGlobalContext;
99-
use crate::{Engine, Module, Trap, Val, ValRaw, module::ModuleRegistry};
99+
use crate::{Engine, Module, Val, ValRaw, module::ModuleRegistry};
100100
use crate::{Global, Instance, Memory, Table, Uninhabited};
101101
use alloc::sync::Arc;
102102
use core::fmt;
@@ -324,6 +324,9 @@ enum CallHookInner<T: 'static> {
324324
/// the deadline for a Store during execution of a function using that store.
325325
#[non_exhaustive]
326326
pub enum UpdateDeadline {
327+
/// Halt execution of WebAssembly, don't update the epoch deadline, and
328+
/// raise a trap.
329+
Interrupt,
327330
/// Extend the deadline by the specified number of ticks.
328331
Continue(u64),
329332
/// Extend the deadline by the specified number of ticks after yielding to
@@ -429,7 +432,7 @@ pub struct StoreOpaque {
429432
// together. Then when we run out of gas, we inject the yield amount from the reserve
430433
// until the reserve is empty.
431434
fuel_reserve: u64,
432-
fuel_yield_interval: Option<NonZeroU64>,
435+
pub(crate) fuel_yield_interval: Option<NonZeroU64>,
433436
/// Indexed data within this `Store`, used to store information about
434437
/// globals, functions, memories, etc.
435438
store_data: StoreData,
@@ -1893,7 +1896,7 @@ impl StoreOpaque {
18931896
Ok(get_fuel(injected_fuel, self.fuel_reserve))
18941897
}
18951898

1896-
fn refuel(&mut self) -> bool {
1899+
pub(crate) fn refuel(&mut self) -> bool {
18971900
let injected_fuel = unsafe { &mut *self.vm_store_context.fuel_consumed.get() };
18981901
refuel(
18991902
injected_fuel,
@@ -2277,6 +2280,22 @@ at https://bytecodealliance.org/security.
22772280

22782281
Ok(id)
22792282
}
2283+
2284+
#[cfg(target_has_atomic = "64")]
2285+
pub(crate) fn set_epoch_deadline(&mut self, delta: u64) {
2286+
// Set a new deadline based on the "epoch deadline delta".
2287+
//
2288+
// Also, note that when this update is performed while Wasm is
2289+
// on the stack, the Wasm will reload the new value once we
2290+
// return into it.
2291+
let current_epoch = self.engine().current_epoch();
2292+
let epoch_deadline = self.vm_store_context.epoch_deadline.get_mut();
2293+
*epoch_deadline = current_epoch + delta;
2294+
}
2295+
2296+
pub(crate) fn get_epoch_deadline(&mut self) -> u64 {
2297+
*self.vm_store_context.epoch_deadline.get_mut()
2298+
}
22802299
}
22812300

22822301
/// Helper parameter to [`StoreOpaque::allocate_instance`].
@@ -2327,67 +2346,19 @@ unsafe impl<T> VMStore for StoreInner<T> {
23272346
)
23282347
}
23292348

2330-
fn out_of_gas(&mut self) -> Result<()> {
2331-
if !self.refuel() {
2332-
return Err(Trap::OutOfFuel.into());
2333-
}
2334-
#[cfg(feature = "async")]
2335-
if self.fuel_yield_interval.is_some() {
2336-
self.async_yield_impl()?;
2337-
}
2338-
Ok(())
2339-
}
2340-
23412349
#[cfg(target_has_atomic = "64")]
2342-
fn new_epoch(&mut self) -> Result<u64, anyhow::Error> {
2350+
fn new_epoch_updated_deadline(&mut self) -> Result<UpdateDeadline> {
23432351
// Temporarily take the configured behavior to avoid mutably borrowing
23442352
// multiple times.
23452353
let mut behavior = self.epoch_deadline_behavior.take();
2346-
let delta_result = match &mut behavior {
2347-
None => Err(Trap::Interrupt.into()),
2348-
Some(callback) => callback((&mut *self).as_context_mut()).and_then(|update| {
2349-
let delta = match update {
2350-
UpdateDeadline::Continue(delta) => delta,
2351-
#[cfg(feature = "async")]
2352-
UpdateDeadline::Yield(delta) => {
2353-
assert!(
2354-
self.async_support(),
2355-
"cannot use `UpdateDeadline::Yield` without enabling async support in the config"
2356-
);
2357-
// Do the async yield. May return a trap if future was
2358-
// canceled while we're yielded.
2359-
self.async_yield_impl()?;
2360-
delta
2361-
}
2362-
#[cfg(feature = "async")]
2363-
UpdateDeadline::YieldCustom(delta, future) => {
2364-
assert!(
2365-
self.async_support(),
2366-
"cannot use `UpdateDeadline::YieldCustom` without enabling async support in the config"
2367-
);
2368-
2369-
// When control returns, we have a `Result<()>` passed
2370-
// in from the host fiber. If this finished successfully then
2371-
// we were resumed normally via a `poll`, so keep going. If
2372-
// the future was dropped while we were yielded, then we need
2373-
// to clean up this fiber. Do so by raising a trap which will
2374-
// abort all wasm and get caught on the other side to clean
2375-
// things up.
2376-
self.block_on(|_| future)?;
2377-
delta
2378-
}
2379-
};
2380-
2381-
// Set a new deadline and return the new epoch deadline so
2382-
// the Wasm code doesn't have to reload it.
2383-
self.set_epoch_deadline(delta);
2384-
Ok(self.get_epoch_deadline())
2385-
})
2354+
let update = match &mut behavior {
2355+
Some(callback) => callback((&mut *self).as_context_mut()),
2356+
None => Ok(UpdateDeadline::Interrupt),
23862357
};
23872358

23882359
// Put back the original behavior which was replaced by `take`.
23892360
self.epoch_deadline_behavior = behavior;
2390-
delta_result
2361+
update
23912362
}
23922363

23932364
#[cfg(feature = "component-model")]
@@ -2397,18 +2368,6 @@ unsafe impl<T> VMStore for StoreInner<T> {
23972368
}
23982369

23992370
impl<T> StoreInner<T> {
2400-
#[cfg(target_has_atomic = "64")]
2401-
pub(crate) fn set_epoch_deadline(&mut self, delta: u64) {
2402-
// Set a new deadline based on the "epoch deadline delta".
2403-
//
2404-
// Also, note that when this update is performed while Wasm is
2405-
// on the stack, the Wasm will reload the new value once we
2406-
// return into it.
2407-
let current_epoch = self.engine().current_epoch();
2408-
let epoch_deadline = self.vm_store_context.epoch_deadline.get_mut();
2409-
*epoch_deadline = current_epoch + delta;
2410-
}
2411-
24122371
#[cfg(target_has_atomic = "64")]
24132372
fn epoch_deadline_trap(&mut self) {
24142373
self.epoch_deadline_behavior = None;
@@ -2421,10 +2380,6 @@ impl<T> StoreInner<T> {
24212380
) {
24222381
self.epoch_deadline_behavior = Some(callback);
24232382
}
2424-
2425-
fn get_epoch_deadline(&mut self) -> u64 {
2426-
*self.vm_store_context.epoch_deadline.get_mut()
2427-
}
24282383
}
24292384

24302385
impl<T: Default> Default for Store<T> {

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -169,21 +169,6 @@ impl<T> StoreInner<T> {
169169

170170
#[doc(hidden)]
171171
impl StoreOpaque {
172-
/// Yields execution to the caller on out-of-gas or epoch interruption.
173-
///
174-
/// This only works on async futures and stores, and assumes that we're
175-
/// executing on a fiber. This will yield execution back to the caller once.
176-
pub fn async_yield_impl(&mut self) -> Result<()> {
177-
// When control returns, we have a `Result<()>` passed
178-
// in from the host fiber. If this finished successfully then
179-
// we were resumed normally via a `poll`, so keep going. If
180-
// the future was dropped while we were yielded, then we need
181-
// to clean up this fiber. Do so by raising a trap which will
182-
// abort all wasm and get caught on the other side to clean
183-
// things up.
184-
self.block_on(|_| Box::pin(crate::runtime::vm::Yield::new()))
185-
}
186-
187172
pub(crate) fn allocate_fiber_stack(&mut self) -> Result<wasmtime_fiber::FiberStack> {
188173
if let Some(stack) = self.async_state.last_fiber_stack().take() {
189174
return Ok(stack);

crates/wasmtime/src/runtime/vm.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -208,16 +208,11 @@ pub unsafe trait VMStore: 'static {
208208
&mut self,
209209
) -> (Option<StoreResourceLimiter<'_>>, &mut StoreOpaque);
210210

211-
/// Callback invoked whenever fuel runs out by a wasm instance. If an error
212-
/// is returned that's raised as a trap. Otherwise wasm execution will
213-
/// continue as normal.
214-
fn out_of_gas(&mut self) -> Result<(), Error>;
215-
216211
/// Callback invoked whenever an instance observes a new epoch
217212
/// number. Cannot fail; cooperative epoch-based yielding is
218213
/// completely semantically transparent. Returns the new deadline.
219214
#[cfg(target_has_atomic = "64")]
220-
fn new_epoch(&mut self) -> Result<u64, Error>;
215+
fn new_epoch_updated_deadline(&mut self) -> Result<crate::UpdateDeadline>;
221216

222217
/// Metadata required for resources for the component model.
223218
#[cfg(feature = "component-model")]

crates/wasmtime/src/runtime/vm/libcalls.rs

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,13 +1211,61 @@ fn memory_atomic_wait64(
12111211

12121212
// Hook for when an instance runs out of fuel.
12131213
fn out_of_gas(store: &mut dyn VMStore, _instance: Pin<&mut Instance>) -> Result<()> {
1214-
store.out_of_gas()
1214+
block_on!(store, async |store| {
1215+
if !store.refuel() {
1216+
return Err(Trap::OutOfFuel.into());
1217+
}
1218+
#[cfg(feature = "async")]
1219+
if store.fuel_yield_interval.is_some() {
1220+
crate::runtime::vm::Yield::new().await;
1221+
}
1222+
Ok(())
1223+
})?
12151224
}
12161225

12171226
// Hook for when an instance observes that the epoch has changed.
12181227
#[cfg(target_has_atomic = "64")]
12191228
fn new_epoch(store: &mut dyn VMStore, _instance: Pin<&mut Instance>) -> Result<NextEpoch> {
1220-
store.new_epoch().map(NextEpoch)
1229+
use crate::UpdateDeadline;
1230+
1231+
let update_deadline = store.new_epoch_updated_deadline()?;
1232+
block_on!(store, async move |store| {
1233+
let delta = match update_deadline {
1234+
UpdateDeadline::Interrupt => return Err(Trap::Interrupt.into()),
1235+
UpdateDeadline::Continue(delta) => delta,
1236+
1237+
// Note that custom assertions for `async_support` are needed below
1238+
// as otherwise if these are used in an
1239+
// `async_support`-disabled-build it'll trip the `assert_ready` part
1240+
// of `block_on!` above. The assertion here provides a more direct
1241+
// error message as to what's going on.
1242+
#[cfg(feature = "async")]
1243+
UpdateDeadline::Yield(delta) => {
1244+
assert!(
1245+
store.async_support(),
1246+
"cannot use `UpdateDeadline::Yield` without enabling \
1247+
async support in the config"
1248+
);
1249+
crate::runtime::vm::Yield::new().await;
1250+
delta
1251+
}
1252+
#[cfg(feature = "async")]
1253+
UpdateDeadline::YieldCustom(delta, future) => {
1254+
assert!(
1255+
store.async_support(),
1256+
"cannot use `UpdateDeadline::YieldCustom` without enabling \
1257+
async support in the config"
1258+
);
1259+
future.await;
1260+
delta
1261+
}
1262+
};
1263+
1264+
// Set a new deadline and return the new epoch deadline so
1265+
// the Wasm code doesn't have to reload it.
1266+
store.set_epoch_deadline(delta);
1267+
Ok(NextEpoch(store.get_epoch_deadline()))
1268+
})?
12211269
}
12221270

12231271
struct NextEpoch(u64);

0 commit comments

Comments
 (0)