c-api: Create RootScope where necessary (#8374) (#8375)

* c-api: Create `RootScope` where necessary

This commit changes the `wasmtime_val_t::{from_val, to_val}` methods to
take a `RootScope` instead of any `AsContextMut`. This then required a
number of refactorings in callers to ensure that a `RootScope` was
created for any function that needed one. This is required to ensure
that GC references in the C API aren't forced to live for the entire
lifetime of the store.

This additionally added `*_unrooted` variants which do the same thing
but don't require `RootScope`. This was needed for when the C API calls
out to the embedder through a function call because a new `RootScope`
wouldn't work for return values (they're bound to a scope within the
closure when we want them to outlive the closure). In these situations
though we know a `RootScope` is already present at the entrypoint.

Closes #8367

* Review comments

crates/c-api/src/async.rs

@@ -7,8 +7,9 @@ use std::pin::Pin;
 use std::sync::Arc;
 use std::task::{Context, Poll};
 use std::{ptr, str};
-
-use wasmtime::{AsContextMut, Func, Instance, Result, StackCreator, StackMemory, Trap, Val};
+use wasmtime::{
+    AsContextMut, Func, Instance, Result, RootScope, StackCreator, StackMemory, Trap, Val,
+};
 
 use crate::{
     bad_utf8, handle_result, to_str, translate_args, wasm_config_t, wasm_functype_t, wasm_trap_t,
@@ -116,7 +117,7 @@ async fn invoke_c_async_callback<'a>(
         params
             .iter()
             .cloned()
-            .map(|p| wasmtime_val_t::from_val(&mut caller, p)),
+            .map(|p| wasmtime_val_t::from_val_unscoped(&mut caller, p)),
     );
     hostcall_val_storage.extend((0..results.len()).map(|_| wasmtime_val_t {
         kind: WASMTIME_I32,
@@ -155,7 +156,7 @@ async fn invoke_c_async_callback<'a>(
     // Translate the `wasmtime_val_t` results into the `results` space
     for (i, result) in out_results.iter().enumerate() {
         unsafe {
-            results[i] = result.to_val(&mut caller.caller);
+            results[i] = result.to_val_unscoped(&mut caller.caller);
         }
     }
     // Move our `vals` storage back into the store now that we no longer
@@ -218,15 +219,14 @@ fn handle_call_error(
 }
 
 async fn do_func_call_async(
-    mut store: WasmtimeStoreContextMut<'_>,
+    mut store: RootScope<WasmtimeStoreContextMut<'_>>,
     func: &Func,
     args: impl ExactSizeIterator<Item = Val>,
     results: &mut [MaybeUninit<wasmtime_val_t>],
     trap_ret: &mut *mut wasm_trap_t,
     err_ret: &mut *mut wasmtime_error_t,
 ) {
-    let mut store = store.as_context_mut();
-    let mut params = mem::take(&mut store.data_mut().wasm_val_storage);
+    let mut params = mem::take(&mut store.as_context_mut().data_mut().wasm_val_storage);
     let (wt_params, wt_results) = translate_args(&mut params, args, results.len());
     let result = func.call_async(&mut store, wt_params, wt_results).await;
 
@@ -236,15 +236,15 @@ async fn do_func_call_async(
                 crate::initialize(slot, wasmtime_val_t::from_val(&mut store, val.clone()));
             }
             params.truncate(0);
-            store.data_mut().wasm_val_storage = params;
+            store.as_context_mut().data_mut().wasm_val_storage = params;
         }
         Err(err) => handle_call_error(err, trap_ret, err_ret),
     }
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn wasmtime_func_call_async<'a>(
-    mut store: WasmtimeStoreContextMut<'a>,
+    store: WasmtimeStoreContextMut<'a>,
     func: &'a Func,
     args: *const wasmtime_val_t,
     nargs: usize,
@@ -253,13 +253,14 @@ pub unsafe extern "C" fn wasmtime_func_call_async<'a>(
     trap_ret: &'a mut *mut wasm_trap_t,
     err_ret: &'a mut *mut wasmtime_error_t,
 ) -> Box<wasmtime_call_future_t<'a>> {
+    let mut scope = RootScope::new(store);
     let args = crate::slice_from_raw_parts(args, nargs)
         .iter()
-        .map(|i| i.to_val(&mut store))
+        .map(|i| i.to_val(&mut scope))
         .collect::<Vec<_>>();
     let results = crate::slice_from_raw_parts_mut(results, nresults);
     let fut = Box::pin(do_func_call_async(
-        store,
+        scope,
         func,
         args.into_iter(),
         results,

crates/c-api/src/func.rs

@@ -3,15 +3,19 @@ use crate::{
     wasmtime_extern_t, wasmtime_val_t, wasmtime_val_union, WasmtimeStoreContext,
     WasmtimeStoreContextMut,
 };
-use crate::{wasm_trap_t, WasmtimeCaller};
+use crate::{wasm_trap_t, WasmtimeCaller, WasmtimeStoreData};
 use anyhow::{Error, Result};
 use std::any::Any;
 use std::ffi::c_void;
 use std::mem::{self, MaybeUninit};
 use std::panic::{self, AssertUnwindSafe};
 use std::ptr;
 use std::str;
-use wasmtime::{AsContextMut, Extern, Func, Trap, Val, ValRaw};
+use wasmtime::{
+    AsContext, AsContextMut, Extern, Func, RootScope, StoreContext, StoreContextMut, Trap, Val,
+    ValRaw,
+};
+
 #[derive(Clone)]
 #[repr(transparent)]
 pub struct wasm_func_t {
@@ -203,6 +207,20 @@ pub struct wasmtime_caller_t<'a> {
     pub(crate) caller: WasmtimeCaller<'a>,
 }
 
+impl AsContext for wasmtime_caller_t<'_> {
+    type Data = WasmtimeStoreData;
+
+    fn as_context(&self) -> StoreContext<'_, WasmtimeStoreData> {
+        self.caller.as_context()
+    }
+}
+
+impl AsContextMut for wasmtime_caller_t<'_> {
+    fn as_context_mut(&mut self) -> StoreContextMut<'_, WasmtimeStoreData> {
+        self.caller.as_context_mut()
+    }
+}
+
 pub type wasmtime_func_callback_t = extern "C" fn(
     *mut c_void,
     *mut wasmtime_caller_t,
@@ -253,7 +271,7 @@ pub(crate) unsafe fn c_callback_to_rust_fn(
             params
                 .iter()
                 .cloned()
-                .map(|p| wasmtime_val_t::from_val(&mut caller, p)),
+                .map(|p| wasmtime_val_t::from_val_unscoped(&mut caller, p)),
         );
         vals.extend((0..results.len()).map(|_| wasmtime_val_t {
             kind: crate::WASMTIME_I32,
@@ -277,7 +295,7 @@ pub(crate) unsafe fn c_callback_to_rust_fn(
 
         // Translate the `wasmtime_val_t` results into the `results` space
         for (i, result) in out_results.iter().enumerate() {
-            results[i] = result.to_val(&mut caller.caller);
+            results[i] = result.to_val_unscoped(&mut caller);
         }
 
         // Move our `vals` storage back into the store now that we no longer
@@ -329,13 +347,13 @@ pub unsafe extern "C" fn wasmtime_func_call(
     nresults: usize,
     trap_ret: &mut *mut wasm_trap_t,
 ) -> Option<Box<wasmtime_error_t>> {
-    let mut store = store.as_context_mut();
-    let mut params = mem::take(&mut store.data_mut().wasm_val_storage);
+    let mut scope = RootScope::new(&mut store);
+    let mut params = mem::take(&mut scope.as_context_mut().data_mut().wasm_val_storage);
     let (wt_params, wt_results) = translate_args(
         &mut params,
         crate::slice_from_raw_parts(args, nargs)
             .iter()
-            .map(|i| i.to_val(&mut store)),
+            .map(|i| i.to_val(&mut scope)),
         nresults,
     );
 
@@ -344,16 +362,16 @@ pub unsafe extern "C" fn wasmtime_func_call(
     // can. As a result we catch panics here and transform them to traps to
     // allow the caller to have any insulation possible against Rust panics.
     let result = panic::catch_unwind(AssertUnwindSafe(|| {
-        func.call(&mut store, wt_params, wt_results)
+        func.call(&mut scope, wt_params, wt_results)
     }));
     match result {
         Ok(Ok(())) => {
             let results = crate::slice_from_raw_parts_mut(results, nresults);
             for (slot, val) in results.iter_mut().zip(wt_results.iter()) {
-                crate::initialize(slot, wasmtime_val_t::from_val(&mut store, val.clone()));
+                crate::initialize(slot, wasmtime_val_t::from_val(&mut scope, val.clone()));
             }
             params.truncate(0);
-            store.data_mut().wasm_val_storage = params;
+            scope.as_context_mut().data_mut().wasm_val_storage = params;
             None
         }
         Ok(Err(trap)) => store_err(trap, trap_ret),

crates/c-api/src/global.rs

@@ -3,7 +3,7 @@ use crate::{
     wasmtime_val_t, WasmtimeStoreContext, WasmtimeStoreContextMut,
 };
 use std::mem::MaybeUninit;
-use wasmtime::{Extern, Global};
+use wasmtime::{Extern, Global, RootScope};
 
 #[derive(Clone)]
 #[repr(transparent)]
@@ -84,8 +84,9 @@ pub unsafe extern "C" fn wasmtime_global_new(
     val: &wasmtime_val_t,
     ret: &mut Global,
 ) -> Option<Box<wasmtime_error_t>> {
-    let val = val.to_val(&mut store);
-    let global = Global::new(store, gt.ty().ty.clone(), val);
+    let mut scope = RootScope::new(&mut store);
+    let val = val.to_val(&mut scope);
+    let global = Global::new(scope, gt.ty().ty.clone(), val);
     handle_result(global, |global| {
         *ret = global;
     })
@@ -101,12 +102,13 @@ pub extern "C" fn wasmtime_global_type(
 
 #[no_mangle]
 pub extern "C" fn wasmtime_global_get(
-    mut store: WasmtimeStoreContextMut<'_>,
+    store: WasmtimeStoreContextMut<'_>,
     global: &Global,
     val: &mut MaybeUninit<wasmtime_val_t>,
 ) {
-    let gval = global.get(&mut store);
-    crate::initialize(val, wasmtime_val_t::from_val(store, gval))
+    let mut scope = RootScope::new(store);
+    let gval = global.get(&mut scope);
+    crate::initialize(val, wasmtime_val_t::from_val(&mut scope, gval))
 }
 
 #[no_mangle]
@@ -115,6 +117,7 @@ pub unsafe extern "C" fn wasmtime_global_set(
     global: &Global,
     val: &wasmtime_val_t,
 ) -> Option<Box<wasmtime_error_t>> {
-    let val = val.to_val(&mut store);
-    handle_result(global.set(store, val), |()| {})
+    let mut scope = RootScope::new(&mut store);
+    let val = val.to_val(&mut scope);
+    handle_result(global.set(scope, val), |()| {})
 }

crates/c-api/src/table.rs

@@ -4,7 +4,7 @@ use crate::{
 };
 use anyhow::anyhow;
 use std::mem::MaybeUninit;
-use wasmtime::{Extern, HeapType, Ref, Table, TableType};
+use wasmtime::{Extern, HeapType, Ref, RootScope, Table, TableType};
 
 #[derive(Clone)]
 #[repr(transparent)]
@@ -121,11 +121,12 @@ pub unsafe extern "C" fn wasmtime_table_new(
     init: &wasmtime_val_t,
     out: &mut Table,
 ) -> Option<Box<wasmtime_error_t>> {
+    let mut scope = RootScope::new(&mut store);
     handle_result(
-        init.to_val(&mut store)
+        init.to_val(&mut scope)
             .ref_()
             .ok_or_else(|| anyhow!("wasmtime_table_new init value is not a reference"))
-            .and_then(|init| Table::new(store, tt.ty().ty.clone(), init)),
+            .and_then(|init| Table::new(scope, tt.ty().ty.clone(), init)),
         |table| *out = table,
     )
 }
@@ -140,14 +141,15 @@ pub unsafe extern "C" fn wasmtime_table_type(
 
 #[no_mangle]
 pub extern "C" fn wasmtime_table_get(
-    mut store: WasmtimeStoreContextMut<'_>,
+    store: WasmtimeStoreContextMut<'_>,
     table: &Table,
     index: u32,
     ret: &mut MaybeUninit<wasmtime_val_t>,
 ) -> bool {
-    match table.get(&mut store, index) {
+    let mut scope = RootScope::new(store);
+    match table.get(&mut scope, index) {
         Some(r) => {
-            crate::initialize(ret, wasmtime_val_t::from_val(store, r.into()));
+            crate::initialize(ret, wasmtime_val_t::from_val(&mut scope, r.into()));
             true
         }
         None => false,
@@ -161,11 +163,12 @@ pub unsafe extern "C" fn wasmtime_table_set(
     index: u32,
     val: &wasmtime_val_t,
 ) -> Option<Box<wasmtime_error_t>> {
+    let mut scope = RootScope::new(&mut store);
     handle_result(
-        val.to_val(&mut store)
+        val.to_val(&mut scope)
             .ref_()
             .ok_or_else(|| anyhow!("wasmtime_table_set value is not a reference"))
-            .and_then(|val| table.set(store, index, val)),
+            .and_then(|val| table.set(scope, index, val)),
         |()| {},
     )
 }
@@ -183,11 +186,12 @@ pub unsafe extern "C" fn wasmtime_table_grow(
     val: &wasmtime_val_t,
     prev_size: &mut u32,
 ) -> Option<Box<wasmtime_error_t>> {
+    let mut scope = RootScope::new(&mut store);
     handle_result(
-        val.to_val(&mut store)
+        val.to_val(&mut scope)
             .ref_()
             .ok_or_else(|| anyhow!("wasmtime_table_grow value is not a reference"))
-            .and_then(|val| table.grow(store, delta, val)),
+            .and_then(|val| table.grow(scope, delta, val)),
         |prev| *prev_size = prev,
     )
 }

crates/c-api/src/val.rs

@@ -5,7 +5,7 @@ use crate::{
 };
 use std::mem::{self, ManuallyDrop, MaybeUninit};
 use std::ptr;
-use wasmtime::{AsContextMut, Func, HeapType, Ref, Val, ValType};
+use wasmtime::{AsContextMut, Func, HeapType, Ref, RootScope, Val, ValType};
 
 #[repr(C)]
 pub struct wasm_val_t {
@@ -177,7 +177,24 @@ pub struct wasmtime_func_t {
 }
 
 impl wasmtime_val_t {
-    pub fn from_val(cx: impl AsContextMut, val: Val) -> wasmtime_val_t {
+    /// Creates a new `wasmtime_val_t` from a `wasmtime::Val`.
+    ///
+    /// Note that this requires a `RootScope` to be present to serve as proof
+    /// that `val` is not require to be rooted in the store itself which would
+    /// prevent GC. Callers should prefer this API where possible, creating a
+    /// temporary `RootScope` when needed.
+    pub fn from_val(cx: &mut RootScope<impl AsContextMut>, val: Val) -> wasmtime_val_t {
+        Self::from_val_unscoped(cx, val)
+    }
+
+    /// Equivalent of [`wasmtime_val_t::from_val`] except that a `RootScope`
+    /// is not required.
+    ///
+    /// This method should only be used when a `RootScope` is known to be
+    /// elsewhere on the stack. For example this is used when we call back out
+    /// to the embedder. In such a situation we know we previously entered with
+    /// some other call so the root scope is on the stack there.
+    pub fn from_val_unscoped(cx: impl AsContextMut, val: Val) -> wasmtime_val_t {
         match val {
             Val::I32(i) => wasmtime_val_t {
                 kind: crate::WASMTIME_I32,
@@ -232,7 +249,22 @@ impl wasmtime_val_t {
         }
     }
 
-    pub unsafe fn to_val(&self, cx: impl AsContextMut) -> Val {
+    /// Convert this `wasmtime_val_t` into a `wasmtime::Val`.
+    ///
+    /// See [`wasmtime_val_t::from_val`] for notes on the `RootScope`
+    /// requirement here. Note that this is particularly meaningful for this
+    /// API as the `Val` returned may contain a `Rooted<T>` which requires a
+    /// `RootScope` if we don't want the value to live for the entire lifetime
+    /// of the `Store`.
+    pub unsafe fn to_val(&self, cx: &mut RootScope<impl AsContextMut>) -> Val {
+        self.to_val_unscoped(cx)
+    }
+
+    /// Equivalent of `to_val` except doesn't require a `RootScope`.
+    ///
+    /// See notes on [`wasmtime_val_t::from_val_unscoped`] for notes on when to
+    /// use this.
+    pub unsafe fn to_val_unscoped(&self, cx: impl AsContextMut) -> Val {
         match self.kind {
             crate::WASMTIME_I32 => Val::I32(self.of.i32),
             crate::WASMTIME_I64 => Val::I64(self.of.i64),
@@ -293,10 +325,11 @@ pub unsafe extern "C" fn wasmtime_val_delete(
 
 #[no_mangle]
 pub unsafe extern "C" fn wasmtime_val_copy(
-    mut cx: WasmtimeStoreContextMut<'_>,
+    cx: WasmtimeStoreContextMut<'_>,
     dst: &mut MaybeUninit<wasmtime_val_t>,
     src: &wasmtime_val_t,
 ) {
-    let val = src.to_val(&mut cx);
-    crate::initialize(dst, wasmtime_val_t::from_val(cx, val))
+    let mut scope = RootScope::new(cx);
+    let val = src.to_val(&mut scope);
+    crate::initialize(dst, wasmtime_val_t::from_val(&mut scope, val))
 }