diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 73d9396962b..43bc48c7615 100644 --- a/azure-pipelines.yml +++ b/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)" diff --git a/crates/cli-support/src/intrinsic.rs b/crates/cli-support/src/intrinsic.rs index f407f8fc12d..6061d82d8ab 100644 --- a/crates/cli-support/src/intrinsic.rs +++ b/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, diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 070ab9fca8b..cbec3136c1a 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/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() diff --git a/crates/cli-support/src/lib.rs b/crates/cli-support/src/lib.rs index 8f15f19dd90..5d1df5c3286 100755 --- a/crates/cli-support/src/lib.rs +++ b/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(); diff --git a/crates/cli/src/bin/wasm-bindgen.rs b/crates/cli/src/bin/wasm-bindgen.rs index 6a633c0b5a3..e40ad50b171 100644 --- a/crates/cli/src/bin/wasm-bindgen.rs +++ b/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, flag_remove_name_section: bool, flag_remove_producers_section: bool, + flag_weak_refs: Option, flag_keep_debug: bool, flag_encode_into: Option, flag_target: Option, @@ -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)?; } diff --git a/crates/js-sys/src/lib.rs b/crates/js-sys/src/lib.rs index 739bc50fe6a..99f1e1b7db8 100644 --- a/crates/js-sys/src/lib.rs +++ b/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 diff --git a/guide/src/SUMMARY.md b/guide/src/SUMMARY.md index 7eb4118f4fc..eb2f3560ac9 100644 --- a/guide/src/SUMMARY.md +++ b/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) diff --git a/guide/src/reference/cli.md b/guide/src/reference/cli.md index 0477d04b34d..477057218a8 100644 --- a/guide/src/reference/cli.md +++ b/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). diff --git a/guide/src/reference/weak-references.md b/guide/src/reference/weak-references.md new file mode 100644 index 00000000000..c8a48eca9ee --- /dev/null +++ b/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. diff --git a/src/closure.rs b/src/closure.rs index 745f02e2d74..c8b81431b70 100644 --- a/src/closure.rs +++ b/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) } } diff --git a/src/lib.rs b/src/lib.rs index d7407397336..7b23789a799 100644 --- a/src/lib.rs +++ b/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;