Fix use-after-free with closures in JS bindings

This commit fixes an erroneous use-after-free which can happen in
erroneous situations in JS. It's intended that if you invoke a closure
after its environment has been destroyed that you'll immediately get an
error from Rust saying so. The JS binding generation for mutable
closures, however, accidentally did not protect against this.

Each closure has an internal reference count which is incremented while
being invoked and decremented when the invocation finishes and also when
the `Closure` in Rust is dropped. That means there's two branches where
the reference count reaches zero and the internal pointer stored in JS
needs to be set to zero. Only one, however, actually set the pointer to
zero!

This means that if a closure was destroyed while it was being invoked it
would not correctly set its internal pointer to zero. A further
invocation of the closure would then pass as seemingly valid pointer
into Rust, causing a use-after-free.

A test isn't included here specifically for this because our CI has
started failing left-and-right over this test, so this commit will
hopefully just make our CI green!

Author: alexcrichton

crates/cli-support/src/js/closures.rs

@@ -181,18 +181,36 @@ impl ClosureDescriptors {
             let mut shim = closure.shim_idx;
             let (js, _ts, _js_doc) = {
                 let mut builder = Js2Rust::new("", input);
+
+                // First up with a closure we increment the internal reference
+                // count. This ensures that the Rust closure environment won't
+                // be deallocated while we're invoking it.
                 builder.prelude("this.cnt++;");
+
                 if closure.mutable {
+                    // For mutable closures they can't be invoked recursively.
+                    // To handle that we swap out the `this.a` pointer with zero
+                    // while we invoke it. If we finish and the closure wasn't
+                    // destroyed, then we put back the pointer so a future
+                    // invocation can succeed.
                     builder
-                        .prelude("let a = this.a;\n")
-                        .prelude("this.a = 0;\n")
+                        .prelude("let a = this.a;")
+                        .prelude("this.a = 0;")
                         .rust_argument("a")
                         .rust_argument("b")
-                        .finally("this.a = a;\n");
+                        .finally("if (this.cnt-- == 1) d(a, b);")
+                        .finally("else this.a = a;");
                 } else {
-                    builder.rust_argument("this.a").rust_argument("b");
+                    // For shared closures they can be invoked recursively so we
+                    // just immediately pass through `this.a`. If we end up
+                    // executing the destructor, however, we clear out the
+                    // `this.a` pointer to prevent it being used again the
+                    // future.
+                    builder
+                        .rust_argument("this.a")
+                        .rust_argument("b")
+                        .finally("if (this.cnt-- == 1) { d(this.a, b); this.a = 0; }");
                 }
-                builder.finally("if (this.cnt-- == 1) d(this.a, b);");
                 builder.process(&closure.function, None)?.finish(
                     "function",
                     "f",