diff --git a/src/closure.rs b/src/closure.rs index 5067c799d..a1fc45c8b 100644 --- a/src/closure.rs +++ b/src/closure.rs @@ -576,7 +576,14 @@ macro_rules! doit { a: usize, b: usize, ) { - debug_assert!(a != 0, "should never destroy a Fn whose pointer is 0"); + // This can be called by the JS glue in erroneous situations + // such as when the closure has already been destroyed. If + // that's the case let's not make things worse by + // segfaulting and/or asserting, so just ignore null + // pointers. + if a == 0 { + return; + } drop(Box::from_raw(FatPtr:: R> { fields: (a, b) }.ptr)); @@ -623,7 +630,10 @@ macro_rules! doit { a: usize, b: usize, ) { - debug_assert!(a != 0, "should never destroy a FnMut whose pointer is 0"); + // See `Fn()` above for why we simply return + if a == 0 { + return; + } drop(Box::from_raw(FatPtr:: R> { fields: (a, b) }.ptr)); @@ -736,7 +746,10 @@ unsafe impl WasmClosure for Fn(&A) -> R a: usize, b: usize, ) { - debug_assert!(a != 0, "should never destroy a Fn whose pointer is 0"); + // See `Fn()` above for why we simply return + if a == 0 { + return; + } drop(Box::from_raw(FatPtr:: R> { fields: (a, b) }.ptr)); @@ -781,7 +794,10 @@ unsafe impl WasmClosure for FnMut(&A) -> R a: usize, b: usize, ) { - debug_assert!(a != 0, "should never destroy a FnMut whose pointer is 0"); + // See `Fn()` above for why we simply return + if a == 0 { + return; + } drop(Box::from_raw(FatPtr:: R> { fields: (a, b) }.ptr)); diff --git a/tests/wasm/closures.js b/tests/wasm/closures.js index 7acaa3832..03b5d59db 100644 --- a/tests/wasm/closures.js +++ b/tests/wasm/closures.js @@ -119,3 +119,7 @@ exports.pass_reference_first_arg_twice = (a, b, c) => { c(a); a.free(); }; + +exports.call_destroyed = f => { + assert.throws(f, /invoked recursively or destroyed/); +}; diff --git a/tests/wasm/closures.rs b/tests/wasm/closures.rs index 1e5af49eb..c6a16f07e 100755 --- a/tests/wasm/closures.rs +++ b/tests/wasm/closures.rs @@ -102,6 +102,7 @@ extern "C" { b: &mut FnMut(&RefFirstArgument), c: &mut FnMut(&RefFirstArgument), ); + fn call_destroyed(a: &JsValue); } #[wasm_bindgen_test] @@ -522,3 +523,37 @@ fn reference_as_first_argument_works2() { ); assert_eq!(a.get(), 2); } + +#[wasm_bindgen_test] +fn call_destroyed_doesnt_segfault() { + struct A(i32, i32); + impl Drop for A { + fn drop(&mut self) { + assert_eq!(self.0, self.1); + } + } + + let a = A(1, 1); + let a = Closure::wrap(Box::new(move || drop(&a)) as Box); + let b = a.as_ref().clone(); + drop(a); + call_destroyed(&b); + + let a = A(2, 2); + let a = Closure::wrap(Box::new(move || drop(&a)) as Box); + let b = a.as_ref().clone(); + drop(a); + call_destroyed(&b); + + let a = A(1, 1); + let a = Closure::wrap(Box::new(move |_: &JsValue| drop(&a)) as Box); + let b = a.as_ref().clone(); + drop(a); + call_destroyed(&b); + + let a = A(2, 2); + let a = Closure::wrap(Box::new(move |_: &JsValue| drop(&a)) as Box); + let b = a.as_ref().clone(); + drop(a); + call_destroyed(&b); +}