Protect against segfaults calling destroyed closures

This commit updates the drop glue generated for closures to simply
ignore null pointers. The drop glue can be called in erroneous
situations such as when a closure is invoked after it's been destroyed.
In these cases we don't want to segfault and/or corrupt memory but
instead let the normal error message from the invoke glue continue to
get propagated.

Closes #1526

Author: alexcrichton

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::<Fn($($var,)*) -> 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::<FnMut($($var,)*) -> R> {
                         fields: (a, b)
                     }.ptr));
@@ -736,7 +746,10 @@ unsafe impl<A, R> 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::<Fn(&A) -> R> {
                 fields: (a, b)
             }.ptr));
@@ -781,7 +794,10 @@ unsafe impl<A, R> 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::<FnMut(&A) -> R> {
                 fields: (a, b)
             }.ptr));

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/);
+};

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<Fn()>);
+    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<FnMut()>);
+    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<Fn(&JsValue)>);
+    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<FnMut(&JsValue)>);
+    let b = a.as_ref().clone();
+    drop(a);
+    call_destroyed(&b);
+}