Skip to content

Commit 8675fa5

Browse files
authored
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!
1 parent a61f068 commit 8675fa5

2 files changed

Lines changed: 36 additions & 0 deletions

File tree

crates/wasmtime/src/func.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,13 +1611,22 @@ macro_rules! impl_into_func {
16111611
)
16121612
}))
16131613
};
1614+
1615+
// Note that we need to be careful when dealing with traps
1616+
// here. Traps are implemented with longjmp/setjmp meaning
1617+
// that it's not unwinding and consequently no Rust
1618+
// destructors are run. We need to be careful to ensure that
1619+
// nothing on the stack needs a destructor when we exit
1620+
// abnormally from this `match`, e.g. on `Err`, on
1621+
// cross-store-issues, or if `Ok(Err)` is raised.
16141622
match ret {
16151623
Err(panic) => wasmtime_runtime::resume_panic(panic),
16161624
Ok(ret) => {
16171625
// Because the wrapped function is not `unsafe`, we
16181626
// can't assume it returned a value that is
16191627
// compatible with this store.
16201628
if !ret.compatible_with_store(weak_store) {
1629+
drop(ret);
16211630
raise_cross_store_trap();
16221631
}
16231632

tests/all/funcref.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
use super::ref_types_module;
2+
use std::cell::Cell;
3+
use std::rc::Rc;
24
use wasmtime::*;
35

46
#[test]
@@ -83,3 +85,28 @@ fn receive_null_funcref_from_wasm() -> anyhow::Result<()> {
8385

8486
Ok(())
8587
}
88+
89+
#[test]
90+
fn wrong_store() -> anyhow::Result<()> {
91+
let dropped = Rc::new(Cell::new(false));
92+
{
93+
let store1 = Store::default();
94+
let store2 = Store::default();
95+
96+
let set = SetOnDrop(dropped.clone());
97+
let f1 = Func::wrap(&store1, move || drop(&set));
98+
let f2 = Func::wrap(&store2, move || Some(f1.clone()));
99+
assert!(f2.call(&[]).is_err());
100+
}
101+
assert!(dropped.get());
102+
103+
return Ok(());
104+
105+
struct SetOnDrop(Rc<Cell<bool>>);
106+
107+
impl Drop for SetOnDrop {
108+
fn drop(&mut self) {
109+
self.0.set(true);
110+
}
111+
}
112+
}

0 commit comments

Comments
 (0)