Fix a memory leak on returning incompatible values (#2424)
This fixes an issue where if a store-incompatible value is returned from a host-defined function then that value is leaked. Practically this means that it's possible to accidentally leak `Func` values, but a simple insertion of a `drop` does the trick!
This commit is contained in:
@@ -1611,6 +1611,14 @@ macro_rules! impl_into_func {
|
|||||||
)
|
)
|
||||||
}))
|
}))
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// Note that we need to be careful when dealing with traps
|
||||||
|
// here. Traps are implemented with longjmp/setjmp meaning
|
||||||
|
// that it's not unwinding and consequently no Rust
|
||||||
|
// destructors are run. We need to be careful to ensure that
|
||||||
|
// nothing on the stack needs a destructor when we exit
|
||||||
|
// abnormally from this `match`, e.g. on `Err`, on
|
||||||
|
// cross-store-issues, or if `Ok(Err)` is raised.
|
||||||
match ret {
|
match ret {
|
||||||
Err(panic) => wasmtime_runtime::resume_panic(panic),
|
Err(panic) => wasmtime_runtime::resume_panic(panic),
|
||||||
Ok(ret) => {
|
Ok(ret) => {
|
||||||
@@ -1618,6 +1626,7 @@ macro_rules! impl_into_func {
|
|||||||
// can't assume it returned a value that is
|
// can't assume it returned a value that is
|
||||||
// compatible with this store.
|
// compatible with this store.
|
||||||
if !ret.compatible_with_store(weak_store) {
|
if !ret.compatible_with_store(weak_store) {
|
||||||
|
drop(ret);
|
||||||
raise_cross_store_trap();
|
raise_cross_store_trap();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
use super::ref_types_module;
|
use super::ref_types_module;
|
||||||
|
use std::cell::Cell;
|
||||||
|
use std::rc::Rc;
|
||||||
use wasmtime::*;
|
use wasmtime::*;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -83,3 +85,28 @@ fn receive_null_funcref_from_wasm() -> anyhow::Result<()> {
|
|||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn wrong_store() -> anyhow::Result<()> {
|
||||||
|
let dropped = Rc::new(Cell::new(false));
|
||||||
|
{
|
||||||
|
let store1 = Store::default();
|
||||||
|
let store2 = Store::default();
|
||||||
|
|
||||||
|
let set = SetOnDrop(dropped.clone());
|
||||||
|
let f1 = Func::wrap(&store1, move || drop(&set));
|
||||||
|
let f2 = Func::wrap(&store2, move || Some(f1.clone()));
|
||||||
|
assert!(f2.call(&[]).is_err());
|
||||||
|
}
|
||||||
|
assert!(dropped.get());
|
||||||
|
|
||||||
|
return Ok(());
|
||||||
|
|
||||||
|
struct SetOnDrop(Rc<Cell<bool>>);
|
||||||
|
|
||||||
|
impl Drop for SetOnDrop {
|
||||||
|
fn drop(&mut self) {
|
||||||
|
self.0.set(true);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user