Deprecate --weak-refs in favor of run-time detection (#3822)

* Deprecate `--weak-refs` in favor of run-time detection

* Generate empty functions instead

* Fix integration tests

* Improve `Closure::into_js_value()` docs

Author: daxpedda

.github/workflows/main.yml

@@ -90,14 +90,7 @@ jobs:
     - run: cargo test --target wasm32-unknown-unknown -p wasm-bindgen-futures
     - run: cargo test --target wasm32-unknown-unknown --test wasm
       env:
-        WASM_BINDGEN_WEAKREF: 1
-    - run: cargo test --target wasm32-unknown-unknown --test wasm
-      env:
-        WASM_BINDGEN_WEAKREF: 1
         WASM_BINDGEN_NO_DEBUG: 1
-    - run: cargo test --target wasm32-unknown-unknown --test wasm --features serde-serialize
-      env:
-        WASM_BINDGEN_WEAKREF: 1
     - run: cargo test --target wasm32-unknown-unknown
       env:
         WASM_BINDGEN_EXTERNREF: 1

CHANGELOG.md

@@ -34,6 +34,9 @@
 * Updated the WebGPU WebIDL to the current draft as of 2024-01-30. Note that this retains the previous update's workaround for `GPUPipelineError`, and holds back an update to the `buffer` argument of the `GPUQueue.{writeBuffer,writeTexture}` methods.
   [#3816](https://github.com/rustwasm/wasm-bindgen/pull/3816)
 
+* Depreate `--weak-refs` and `WASM_BINDGEN_WEAKREF` in favor of automatic run-time detection.
+  [#3822](https://github.com/rustwasm/wasm-bindgen/pull/3822)
+
 ### Fixed
 
 * Fixed UB when freeing strings received from JS if not using the default allocator.

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

@@ -921,18 +921,12 @@ impl<'a> Context<'a> {
                 "
                 static __wrap(ptr) {{
                     ptr = ptr >>> 0;
-                    const obj = Object.create({}.prototype);
+                    const obj = Object.create({name}.prototype);
                     obj.__wbg_ptr = ptr;
-                    {}
+                    {name}Finalization.register(obj, obj.__wbg_ptr, obj);
                     return obj;
                 }}
-                ",
-                name,
-                if self.config.weak_refs {
-                    format!("{}Finalization.register(obj, obj.__wbg_ptr, obj);", name)
-                } else {
-                    String::new()
-                },
+                "
             ));
         }
 
@@ -950,13 +944,13 @@ impl<'a> Context<'a> {
             ));
         }
 
-        if self.config.weak_refs {
-            self.global(&format!(
-                "const {}Finalization = new FinalizationRegistry(ptr => wasm.{}(ptr >>> 0));",
-                name,
-                wasm_bindgen_shared::free_function(name),
-            ));
-        }
+        self.global(&format!(
+            "
+            const {name}Finalization = (typeof FinalizationRegistry === 'undefined')
+                ? {{ register: () => {{}}, unregister: () => {{}} }}
+                : new FinalizationRegistry(ptr => wasm.{}(ptr >>> 0));",
+            wasm_bindgen_shared::free_function(name),
+        ));
 
         // If the class is inspectable, generate `toJSON` and `toString`
         // to expose all readable properties of the class. Otherwise,
@@ -1021,7 +1015,7 @@ impl<'a> Context<'a> {
             __destroy_into_raw() {{
                 const ptr = this.__wbg_ptr;
                 this.__wbg_ptr = 0;
-                {}
+                {name}Finalization.unregister(this);
                 return ptr;
             }}
 
@@ -1030,11 +1024,6 @@ impl<'a> Context<'a> {
                 wasm.{}(ptr);
             }}
             ",
-            if self.config.weak_refs {
-                format!("{}Finalization.unregister(this);", name)
-            } else {
-                String::new()
-            },
             wasm_bindgen_shared::free_function(name),
         ));
         ts_dst.push_str("  free(): void;\n");
@@ -2121,15 +2110,7 @@ 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 {
-            ("", "")
-        };
+        self.expose_closure_finalization()?;
 
         // For mutable closures they can't be invoked recursively.
         // To handle that we swap out the `this.a` pointer with zero
@@ -2152,20 +2133,17 @@ impl<'a> Context<'a> {
                     }} finally {{
                         if (--state.cnt === 0) {{
                             wasm.{table}.get(state.dtor)(a, state.b);
-                            {unregister}
+                            CLOSURE_DTORS.unregister(state);
                         }} else {{
                             state.a = a;
                         }}
                     }}
                 }};
                 real.original = state;
-                {register}
+                CLOSURE_DTORS.register(real, state, state);
                 return real;
             }}
             ",
-            table = table,
-            register = register,
-            unregister = unregister,
         ));
 
         Ok(())
@@ -2178,15 +2156,7 @@ 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 {
-            ("", "")
-        };
+        self.expose_closure_finalization()?;
 
         // For shared closures they can be invoked recursively so we
         // just immediately pass through `this.a`. If we end up
@@ -2208,18 +2178,15 @@ impl<'a> Context<'a> {
                         if (--state.cnt === 0) {{
                             wasm.{table}.get(state.dtor)(state.a, state.b);
                             state.a = 0;
-                            {unregister}
+                            CLOSURE_DTORS.unregister(state);
                         }}
                     }}
                 }};
                 real.original = state;
-                {register}
+                CLOSURE_DTORS.register(real, state, state);
                 return real;
             }}
             ",
-            table = table,
-            register = register,
-            unregister = unregister,
         ));
 
         Ok(())
@@ -2229,15 +2196,15 @@ impl<'a> Context<'a> {
         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
+            const CLOSURE_DTORS = (typeof FinalizationRegistry === 'undefined')
+                ? {{ register: () => {{}}, unregister: () => {{}} }}
+                : new FinalizationRegistry(state => {{
+                    wasm.{table}.get(state.dtor)(state.a, state.b)
+                }});
+            "
         ));
 
         Ok(())

crates/cli-support/src/lib.rs

@@ -35,9 +35,6 @@ pub struct Bindgen {
     remove_producers_section: bool,
     omit_default_module_path: bool,
     emit_start: bool,
-    // Experimental support for weakrefs, an upcoming ECMAScript feature.
-    // Currently only enable-able through an env var.
-    weak_refs: bool,
     // Support for the wasm threads proposal, transforms the wasm module to be
     // "ready to be instantiated on any thread"
     threads: wasm_bindgen_threads_xform::Config,
@@ -106,7 +103,6 @@ impl Bindgen {
             remove_name_section: false,
             remove_producers_section: false,
             emit_start: true,
-            weak_refs: env::var("WASM_BINDGEN_WEAKREF").is_ok(),
             threads: threads_config(),
             externref,
             multi_value,
@@ -126,11 +122,6 @@ impl Bindgen {
         self
     }
 
-    pub fn weak_refs(&mut self, enable: bool) -> &mut Bindgen {
-        self.weak_refs = enable;
-        self
-    }
-
     pub fn reference_types(&mut self, enable: bool) -> &mut Bindgen {
         self.externref = enable;
         self

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

@@ -39,7 +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
+    --weak-refs                  Deprecated, is runtime-detected
     --reference-types            Enable usage of WebAssembly reference types
     -V --version                 Print the version number of wasm-bindgen
 
@@ -126,9 +126,6 @@ fn rmain(args: &Args) -> Result<(), Error> {
         .omit_imports(args.flag_omit_imports)
         .omit_default_module_path(args.flag_omit_default_module_path)
         .split_linked_modules(args.flag_split_linked_modules);
-    if let Some(true) = args.flag_weak_refs {
-        b.weak_refs(true);
-    }
     if let Some(true) = args.flag_reference_types {
         b.reference_types(true);
     }

crates/cli/tests/reference/builder.js

@@ -23,6 +23,10 @@ function getStringFromWasm0(ptr, len) {
     ptr = ptr >>> 0;
     return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len));
 }
+
+const ClassBuilderFinalization = (typeof FinalizationRegistry === 'undefined')
+    ? { register: () => {}, unregister: () => {} }
+    : new FinalizationRegistry(ptr => wasm.__wbg_classbuilder_free(ptr >>> 0));
 /**
 */
 export class ClassBuilder {
@@ -31,14 +35,14 @@ export class ClassBuilder {
         ptr = ptr >>> 0;
         const obj = Object.create(ClassBuilder.prototype);
         obj.__wbg_ptr = ptr;
-
+        ClassBuilderFinalization.register(obj, obj.__wbg_ptr, obj);
         return obj;
     }
 
     __destroy_into_raw() {
         const ptr = this.__wbg_ptr;
         this.__wbg_ptr = 0;
-
+        ClassBuilderFinalization.unregister(this);
         return ptr;
     }
 

crates/cli/tests/reference/constructor.js

@@ -23,14 +23,18 @@ function getStringFromWasm0(ptr, len) {
     ptr = ptr >>> 0;
     return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len));
 }
+
+const ClassConstructorFinalization = (typeof FinalizationRegistry === 'undefined')
+    ? { register: () => {}, unregister: () => {} }
+    : new FinalizationRegistry(ptr => wasm.__wbg_classconstructor_free(ptr >>> 0));
 /**
 */
 export class ClassConstructor {
 
     __destroy_into_raw() {
         const ptr = this.__wbg_ptr;
         this.__wbg_ptr = 0;
-
+        ClassConstructorFinalization.unregister(this);
         return ptr;
     }
 

guide/src/reference/cli.md

@@ -84,15 +84,6 @@ 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).
-
 ### `--reference-types`
 
 Enables usage of the [WebAssembly References Types

guide/src/reference/weak-references.md

@@ -1,9 +1,8 @@
 # Support for Weak References
 
-By default wasm-bindgen does not use 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.
+By default wasm-bindgen does use the [TC39 weak references
+proposal](https://github.com/tc39/proposal-weakrefs) if support is detected.
+At the time of this writing all major browsers do support it.
 
 Without weak references your JS integration may be susceptible to memory leaks
 in Rust, for example:
@@ -15,9 +14,8 @@ in Rust, for example:
 * Rust closures have `Closure::{into_js_value,forget}` methods which explicitly
   do not 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
+These issues are all solved with the weak references proposal in JS.
+`FinalizationRegistry` will 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.
+still be automatically deallocated if `FinalizationRegistry` support is detected.

src/closure.rs

@@ -356,16 +356,12 @@ where
     /// lifetime dynamically managed by the JS GC. This function can be used
     /// to drop this `Closure` while keeping the associated JS function still
     /// valid.
-    ///
-    /// 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.
+    /// 
+    /// If the platform supports weak references, the Rust memory will be
+    /// reclaimed when the JS closure is GC'd. If weak references is not
+    /// supported, 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.
     pub fn into_js_value(self) -> JsValue {
         let idx = self.js.idx;
         mem::forget(self);