From 0c922906d3563a57bc65e8b002d0626b297bd018 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 22 Apr 2024 14:56:20 -0500 Subject: [PATCH] Fix a panic calling host functions with refs in async mode (#8434) (#8439) This commit fixes a panic when a host function defined with `Func::new` returned GC references and was called in async mode. The logic to auto-gc before the return values go to wasm asserted that a synchronous GC was possible but the context this function is called in could be either async or sync. The fix applied in this commit is to remove the auto-gc. This means that hosts will need to explicitly GC in these situations until auto-gc is re-added back to Wasmtime. cc #8433 as this will make the behavior consistent, but we'll want to re-add the gc behavior. --- crates/wasmtime/src/runtime/func.rs | 18 --------- tests/all/async_functions.rs | 62 +++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 18 deletions(-) diff --git a/crates/wasmtime/src/runtime/func.rs b/crates/wasmtime/src/runtime/func.rs index a02c18f2e19d..a06ce4e3b008 100644 --- a/crates/wasmtime/src/runtime/func.rs +++ b/crates/wasmtime/src/runtime/func.rs @@ -1345,24 +1345,6 @@ impl Func { let (params, results) = val_vec.split_at_mut(nparams); func(caller.sub_caller(), params, results)?; - #[cfg(feature = "gc")] - { - // See the comment in `Func::call_impl_check_args`. - let num_gc_refs = ty.as_wasm_func_type().non_i31_gc_ref_returns_count(); - if let Some(num_gc_refs) = NonZeroUsize::new(num_gc_refs) { - if caller - .as_context() - .0 - .gc_store()? - .gc_heap - .need_gc_before_entering_wasm(num_gc_refs) - { - assert!(!caller.as_context().0.async_support()); - caller.as_context_mut().gc(); - } - } - } - // Unlike our arguments we need to dynamically check that the return // values produced are correct. There could be a bug in `func` that // produces the wrong number, wrong types, or wrong stores of diff --git a/tests/all/async_functions.rs b/tests/all/async_functions.rs index 5e1fbf17daea..2eaca31f6af2 100644 --- a/tests/all/async_functions.rs +++ b/tests/all/async_functions.rs @@ -983,3 +983,65 @@ async fn gc_preserves_externref_on_historical_async_stacks() -> Result<()> { Ok(()) } + +#[tokio::test] +async fn async_gc_with_func_new_and_func_wrap() -> Result<()> { + let _ = env_logger::try_init(); + + let mut config = Config::new(); + config.async_support(true); + let engine = Engine::new(&config)?; + + let module = Module::new( + &engine, + r#" + (module $m1 + (import "" "a" (func $a (result externref))) + (import "" "b" (func $b (result externref))) + + (table 2 funcref) + (elem (i32.const 0) func $a $b) + + (func (export "a") + (call $call (i32.const 0)) + ) + (func (export "b") + (call $call (i32.const 1)) + ) + + (func $call (param i32) + (local $cnt i32) + (loop $l + (drop (call_indirect (result externref) (local.get 0))) + (local.set $cnt (i32.add (local.get $cnt) (i32.const 1))) + + (if (i32.lt_u (local.get $cnt) (i32.const 1000)) + (then (br $l))) + ) + ) + ) + "#, + )?; + + let mut linker = Linker::new(&engine); + linker.func_wrap("", "a", |mut cx: Caller<'_, _>| { + Ok(Some(ExternRef::new(&mut cx, 100)?)) + })?; + let ty = FuncType::new(&engine, [], [ValType::EXTERNREF]); + linker.func_new("", "b", ty, |mut cx, _, results| { + results[0] = ExternRef::new(&mut cx, 100)?.into(); + Ok(()) + })?; + + let mut store = Store::new(&engine, ()); + let instance = linker.instantiate_async(&mut store, &module).await?; + let a = instance.get_typed_func::<(), ()>(&mut store, "a")?; + a.call_async(&mut store, ()).await?; + + let mut store = Store::new(&engine, ()); + let instance = linker.instantiate_async(&mut store, &module).await?; + let b = instance.get_typed_func::<(), ()>(&mut store, "b")?; + b.call_async(&mut store, ()).await?; + + Ok(()) +}