Update support for weak references (#2248)

* Update support for weak referenes

This commit updates the `WASM_BINDGEN_WEAKREF` feature support to the
latest version of the weak references proposal in JS. This also updates
the `Closure` type to use finalization registries to ensure closures are
deallocated with the JS gc. This means that `Closure::forget` will not
actually leak memory in a weakref-using application.

* Add a flag for weak references

azure-pipelines.yml

@@ -48,6 +48,17 @@ jobs:
       #   displayName: "(anyref) Crate test suite (no debug)"
       # - script: cargo test --target wasm32-unknown-unknown --features serde-serialize --test wasm
       #   displayName: "(anyref) Crate test suite (with serde)"
+      - script: |
+          set -e
+          echo "##vso[task.setvariable variable=NODE_ARGS]--harmony-weak-refs"
+          echo "##vso[task.setvariable variable=WASM_BINDGEN_WEAKREF]1"
+        displayName: "Configure anyref passes"
+      - script: cargo test --target wasm32-unknown-unknown --test wasm
+        displayName: "(weakref) Crate test suite"
+      - script: WASM_BINDGEN_NO_DEBUG=1 cargo test --target wasm32-unknown-unknown --test wasm
+        displayName: "(weakref) Crate test suite (no debug)"
+      - script: cargo test --target wasm32-unknown-unknown --features serde-serialize --test wasm
+        displayName: "(weakref) Crate test suite (with serde)"
 
   - job: test_wasm_bindgen_windows
     displayName: "Run wasm-bindgen crate tests (Windows)"

crates/cli-support/src/intrinsic.rs

@@ -114,9 +114,6 @@ intrinsics! {
         #[symbol = "__wbindgen_cb_drop"]
         #[signature = fn(Externref) -> Boolean]
         CallbackDrop,
-        #[symbol = "__wbindgen_cb_forget"]
-        #[signature = fn(Externref) -> Unit]
-        CallbackForget,
         #[symbol = "__wbindgen_number_new"]
         #[signature = fn(F64) -> Externref]
         NumberNew,

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

@@ -781,7 +781,7 @@ impl<'a> Context<'a> {
                 ",
                 name,
                 if self.config.weak_refs {
-                    format!("{}FinalizationGroup.register(obj, obj.ptr, obj.ptr);", name)
+                    format!("{}Finalization.register(obj, obj.ptr, obj);", name)
                 } else {
                     String::new()
                 },
@@ -790,13 +790,7 @@ impl<'a> Context<'a> {
 
         if self.config.weak_refs {
             self.global(&format!(
-                "
-                const {}FinalizationGroup = new FinalizationGroup((items) => {{
-                    for (const ptr of items) {{
-                        wasm.{}(ptr);
-                    }}
-                }});
-                ",
+                "const {}Finalization = new FinalizationRegistry(ptr => wasm.{}(ptr));",
                 name,
                 wasm_bindgen_shared::free_function(&name),
             ));
@@ -860,7 +854,7 @@ impl<'a> Context<'a> {
             }}
             ",
             if self.config.weak_refs {
-                format!("{}FinalizationGroup.unregister(ptr);", name)
+                format!("{}Finalization.unregister(this);", name)
             } else {
                 String::new()
             },
@@ -1938,6 +1932,16 @@ impl<'a> Context<'a> {
 
         let table = self.export_function_table()?;
 
+        let (register, unregister) = if self.config.weak_refs {
+            self.expose_closure_finalization()?;
+            (
+                "CLOSURE_DTORS.register(real, state, state);",
+                "CLOSURE_DTORS.unregister(state)",
+            )
+        } else {
+            ("", "")
+        };
+
         // 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
@@ -1946,7 +1950,7 @@ impl<'a> Context<'a> {
         self.global(&format!(
             "
             function makeMutClosure(arg0, arg1, dtor, f) {{
-                const state = {{ a: arg0, b: arg1, cnt: 1 }};
+                const state = {{ a: arg0, b: arg1, cnt: 1, dtor }};
                 const real = (...args) => {{
                     // First up with a closure we increment the internal reference
                     // count. This ensures that the Rust closure environment won't
@@ -1957,15 +1961,22 @@ impl<'a> Context<'a> {
                     try {{
                         return f(a, state.b, ...args);
                     }} finally {{
-                        if (--state.cnt === 0) wasm.{}.get(dtor)(a, state.b);
-                        else state.a = a;
+                        if (--state.cnt === 0) {{
+                            wasm.{table}.get(state.dtor)(a, state.b);
+                            {unregister}
+                        }} else {{
+                            state.a = a;
+                        }}
                     }}
                 }};
                 real.original = state;
+                {register}
                 return real;
             }}
             ",
-            table
+            table = table,
+            register = register,
+            unregister = unregister,
         ));
 
         Ok(())
@@ -1978,6 +1989,16 @@ impl<'a> Context<'a> {
 
         let table = self.export_function_table()?;
 
+        let (register, unregister) = if self.config.weak_refs {
+            self.expose_closure_finalization()?;
+            (
+                "CLOSURE_DTORS.register(real, state, state);",
+                "CLOSURE_DTORS.unregister(state)",
+            )
+        } else {
+            ("", "")
+        };
+
         // 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
@@ -1986,7 +2007,7 @@ impl<'a> Context<'a> {
         self.global(&format!(
             "
             function makeClosure(arg0, arg1, dtor, f) {{
-                const state = {{ a: arg0, b: arg1, cnt: 1 }};
+                const state = {{ a: arg0, b: arg1, cnt: 1, dtor }};
                 const real = (...args) => {{
                     // First up with a closure we increment the internal reference
                     // count. This ensures that the Rust closure environment won't
@@ -1996,21 +2017,42 @@ impl<'a> Context<'a> {
                         return f(state.a, state.b, ...args);
                     }} finally {{
                         if (--state.cnt === 0) {{
-                            wasm.{}.get(dtor)(state.a, state.b);
+                            wasm.{table}.get(state.dtor)(state.a, state.b);
                             state.a = 0;
+                            {unregister}
                         }}
                     }}
                 }};
                 real.original = state;
+                {register}
                 return real;
             }}
             ",
-            table
+            table = table,
+            register = register,
+            unregister = unregister,
         ));
 
         Ok(())
     }
 
+    fn expose_closure_finalization(&mut self) -> Result<(), Error> {
+        if !self.should_write_global("closure_finalization") {
+            return Ok(());
+        }
+        assert!(self.config.weak_refs);
+        let table = self.export_function_table()?;
+        self.global(&format!(
+            "
+            const CLOSURE_DTORS = new FinalizationRegistry(state => {{
+                wasm.{}.get(state.dtor)(state.a, state.b)
+            }});
+            ",
+            table
+        ));
+
+        Ok(())
+    }
     fn global(&mut self, s: &str) {
         let s = s.trim();
 
@@ -2893,11 +2935,6 @@ impl<'a> Context<'a> {
                 "false".to_string()
             }
 
-            Intrinsic::CallbackForget => {
-                assert_eq!(args.len(), 1);
-                args[0].clone()
-            }
-
             Intrinsic::NumberNew => {
                 assert_eq!(args.len(), 1);
                 args[0].clone()

crates/cli-support/src/lib.rs

@@ -128,6 +128,11 @@ impl Bindgen {
         self
     }
 
+    pub fn weak_refs(&mut self, enable: bool) -> &mut Bindgen {
+        self.weak_refs = enable;
+        self
+    }
+
     /// Explicitly specify the already parsed input module.
     pub fn input_module(&mut self, name: &str, module: Module) -> &mut Bindgen {
         let name = name.to_string();

crates/cli/src/bin/wasm-bindgen.rs

@@ -39,6 +39,7 @@ Options:
     --nodejs                     Deprecated, use `--target nodejs`
     --web                        Deprecated, use `--target web`
     --no-modules                 Deprecated, use `--target no-modules`
+    --weak-refs                  Enable usage of the JS weak references proposal
     -V --version                 Print the version number of wasm-bindgen
 ";
 
@@ -59,6 +60,7 @@ struct Args {
     flag_no_modules_global: Option<String>,
     flag_remove_name_section: bool,
     flag_remove_producers_section: bool,
+    flag_weak_refs: Option<bool>,
     flag_keep_debug: bool,
     flag_encode_into: Option<String>,
     flag_target: Option<String>,
@@ -114,6 +116,9 @@ fn rmain(args: &Args) -> Result<(), Error> {
         .remove_producers_section(args.flag_remove_producers_section)
         .typescript(typescript)
         .omit_imports(args.flag_omit_imports);
+    if flags.flag_weak_refs {
+        b.weak_refs(true);
+    }
     if let Some(ref name) = args.flag_no_modules_global {
         b.no_modules_global(name)?;
     }

crates/js-sys/src/lib.rs

@@ -778,6 +778,7 @@ extern "C" {
     /// [MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)
     #[wasm_bindgen(constructor)]
     #[deprecated(note = "recommended to use `Boolean::from` instead")]
+    #[allow(deprecated)]
     pub fn new(value: &JsValue) -> Boolean;
 
     /// The `valueOf()` method returns the primitive value of a `Boolean` object.
@@ -1843,6 +1844,7 @@ extern "C" {
     /// [MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number)
     #[wasm_bindgen(constructor)]
     #[deprecated(note = "recommended to use `Number::from` instead")]
+    #[allow(deprecated)]
     pub fn new(value: &JsValue) -> Number;
 
     /// The `Number.parseInt()` method parses a string argument and returns an

guide/src/SUMMARY.md

@@ -41,6 +41,7 @@
   - [Optimizing for Size](./reference/optimize-size.md)
   - [Supported Rust Targets](./reference/rust-targets.md)
   - [Supported Browsers](./reference/browser-support.md)
+  - [Support for Weak References](./reference/weak-references.md)
   - [Supported Types](./reference/types.md)
     - [Imported JavaScript Types](./reference/types/imported-js-types.md)
     - [Exported Rust Types](./reference/types/exported-rust-types.md)

guide/src/reference/cli.md

@@ -13,23 +13,23 @@ always be listed via `wasm-bindgen --help`.
 
 The recommend way to install the `wasm-bindgen` command line tool is with the
 `wasm-pack` installer described
-[here](https://rustwasm.github.io/wasm-pack/installer/). After installing 
+[here](https://rustwasm.github.io/wasm-pack/installer/). After installing
 `wasm-pack`, you are ready to build project invoking `wasm-pack build`.
 This command installs apropriate version of the `wasm-bindgen` command-line
 tool. The version of `wasm-bindgen` installed by `wasm-pack` is not available
  to be used directly via command line.
 
-It is not recommended to install `wasm-bindgen-cli` as its version must match 
-_exactly_ the version of `wasm-bindgen` that is specified in the project's 
-cargo.lock file. Using `wasm-pack` for building simplifies the build process 
-as `wasm-pack` ensures that the proper version of `wasm-bindgen` command-line 
-tool is used. That means that `wasm-pack` may install many different versions 
-of `wasm-bindgen`, but during the build `wasm-pack` will always make sure to 
+It is not recommended to install `wasm-bindgen-cli` as its version must match
+_exactly_ the version of `wasm-bindgen` that is specified in the project's
+cargo.lock file. Using `wasm-pack` for building simplifies the build process
+as `wasm-pack` ensures that the proper version of `wasm-bindgen` command-line
+tool is used. That means that `wasm-pack` may install many different versions
+of `wasm-bindgen`, but during the build `wasm-pack` will always make sure to
 use the correct one.
 
-Note: if, for any reason, you decide to use wasm-bindgen directly (this is 
-not recommended!) you will have to manually take care of using exactly the 
-same version of wasm-bindgen command-line tool (wasm-bindgen-cli) that 
+Note: if, for any reason, you decide to use wasm-bindgen directly (this is
+not recommended!) you will have to manually take care of using exactly the
+same version of wasm-bindgen command-line tool (wasm-bindgen-cli) that
 matches the version of wasm-bingden in cargo.lock.
 
 
@@ -101,3 +101,12 @@ sections.
 When generating bundler-compatible code (see the section on [deployment]) this
 indicates that the bundled code is always intended to go into a browser so a few
 checks for Node.js can be elided.
+
+### `--weak-refs`
+
+Enables usage of the [TC39 Weak References
+proposal](https://github.com/tc39/proposal-weakrefs), ensuring that all Rust
+memory is eventually deallocated regardless of whether you're calling `free` or
+not. This is off-by-default while we're waiting for support to percolate into
+all major browsers. For more information see the [documentation about weak
+references](./weak-references.md).

guide/src/reference/weak-references.md

@@ -0,0 +1,23 @@
+# Support for Weak References
+
+By default wasm-bindgen does not uses the [TC39 weak references
+proposal](https://github.com/tc39/proposal-weakrefs). This proposal just
+advanced to stage 4 at the time of this writing, but it will still stake some
+time for support to percolate into all the major browsers.
+
+Without weak references your JS integration may be susceptible to memory leaks
+in Rust, for example:
+
+* You could forget to call `.free()` on a JS object, leaving the Rust memory
+  allocated.
+* Rust closures converted to JS values (the `Closure` type) may not be executed
+  and cleaned up.
+* Rust closures have a `Closure::forget` method which explicitly doesn't free
+  the underlying memory.
+
+These issues are all solved with the weak references proposal in JS. The
+`--weak-refs` flag to the `wasm-bindgen` CLI will enable usage of
+`FinalizationRegistry` to ensure that all memory is cleaned up, regardless of
+whether it's explicitly deallocated or not. Note that explicit deallocation
+is always a possibility and supported, but if it's not called then memory will
+still be automatically deallocated with the `--weak-refs` flag.

src/closure.rs

@@ -356,22 +356,28 @@ where
         }
     }
 
-    /// Leaks this `Closure` to ensure it remains valid for the duration of the
-    /// entire program.
+    /// Release memory management of this closure from Rust to the JS GC.
     ///
-    /// > **Note**: this function will leak memory. It should be used sparingly
-    /// > to ensure the memory leak doesn't affect the program too much.
+    /// When a `Closure` is dropped it will release the Rust memory and
+    /// invalidate the associated JS closure, but this isn't always desired.
+    /// Some callbacks are alive for the entire duration of the program or for a
+    /// lifetime dynamically managed by the JS GC. This function can be used
+    /// to drop this `Closure` while keeping the associated JS function still
+    /// valid.
     ///
-    /// When a `Closure` is dropped it will invalidate the associated JS
-    /// closure, but this isn't always desired. Some callbacks are alive for
-    /// the entire duration of the program, so this can be used to conveniently
-    /// leak this instance of `Closure` while performing as much internal
-    /// cleanup as it can.
-    pub fn forget(self) {
-        unsafe {
-            super::__wbindgen_cb_forget(self.js.idx);
-            mem::forget(self);
-        }
+    /// By default this function will leak memory. This can be dangerous if this
+    /// function is called many times in an application because the memory leak
+    /// will overwhelm the page quickly and crash the wasm.
+    ///
+    /// If the browser, however, supports weak references, then this function
+    /// will not leak memory. Instead the Rust memory will be reclaimed when the
+    /// JS closure is GC'd. Weak references are not enabled by default since
+    /// they're still a proposal for the JS standard. They can be enabled with
+    /// `WASM_BINDGEN_WEAKREF=1` when running `wasm-bindgen`, however.
+    pub fn forget(self) -> JsValue {
+        let idx = self.js.idx;
+        mem::forget(self);
+        JsValue::_new(idx)
     }
 }
 

src/lib.rs

@@ -515,7 +515,6 @@ externs! {
         fn __wbindgen_rethrow(a: u32) -> !;
 
         fn __wbindgen_cb_drop(idx: u32) -> u32;
-        fn __wbindgen_cb_forget(idx: u32) -> ();
 
         fn __wbindgen_describe(v: u32) -> ();
         fn __wbindgen_describe_closure(a: u32, b: u32, c: u32) -> u32;