From e13172566850cb9e4d507d56825a68a1e0846cc6 Mon Sep 17 00:00:00 2001 From: chemicstry Date: Wed, 7 Oct 2020 00:35:23 +0300 Subject: [PATCH 1/5] Fix multithreaded wasm crash --- Cargo.toml | 3 ++- src/wasm-bindgen.rs | 37 +++++++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5bb3f0dae..8f43e16bb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ wasi = "0.10" stdweb = { version = "0.4.18", default-features = false, optional = true } [target.'cfg(all(target_arch = "wasm32", target_os = "unknown", not(cargo_web)))'.dependencies] wasm-bindgen = { version = "0.2.62", default-features = false, optional = true } +js-sys = { version = "0.3", optional = true } [target.'cfg(all(target_arch = "wasm32", target_os = "unknown", not(cargo_web)))'.dev-dependencies] wasm-bindgen-test = "0.3.18" @@ -40,7 +41,7 @@ std = [] # Feature to enable fallback RDRAND-based implementation on x86/x86_64 rdrand = [] # Feature to enable JavaScript bindings on wasm32-unknown-unknown -js = ["stdweb", "wasm-bindgen"] +js = ["stdweb", "wasm-bindgen", "js-sys"] # Feature to enable custom RNG implementations custom = [] # Unstable feature to support being a libstd dependency diff --git a/src/wasm-bindgen.rs b/src/wasm-bindgen.rs index 4e7848508..0f33843cd 100644 --- a/src/wasm-bindgen.rs +++ b/src/wasm-bindgen.rs @@ -11,10 +11,20 @@ extern crate std; use std::thread_local; use wasm_bindgen::prelude::*; +use js_sys::Uint8Array; + +// Maximum is 65536 bytes see https://developer.mozilla.org/en-US/docs/Web/API/Crypto/getRandomValues +const BROWSER_CRYPTO_BUFFER_SIZE: usize = 256; + +struct BrowserCryptoContext { + crypto: BrowserCrypto, + // A temporary buffer backed by browser memory to avoid multithreaded wasm exception. See issue #164 + buf: Uint8Array, +} enum RngSource { Node(NodeCrypto), - Browser(BrowserCrypto), + Browser(BrowserCryptoContext), } // JsValues are always per-thread, so we initialize RngSource for each thread. @@ -33,17 +43,12 @@ pub(crate) fn getrandom_inner(dest: &mut [u8]) -> Result<(), Error> { return Err(Error::NODE_RANDOM_FILL_SYNC); } } - RngSource::Browser(n) => { - // see https://developer.mozilla.org/en-US/docs/Web/API/Crypto/getRandomValues - // - // where it says: - // - // > A QuotaExceededError DOMException is thrown if the - // > requested length is greater than 65536 bytes. - for chunk in dest.chunks_mut(65536) { - if n.get_random_values(chunk).is_err() { + RngSource::Browser(ctx) => { + for chunk in dest.chunks_mut(BROWSER_CRYPTO_BUFFER_SIZE) { + if ctx.crypto.get_random_values(&ctx.buf).is_err() { return Err(Error::WEB_GET_RANDOM_VALUES); } + ctx.buf.copy_to(chunk); } } }; @@ -63,7 +68,15 @@ fn getrandom_init() -> Result { (_, crypto) if !crypto.is_undefined() => crypto, _ => return Err(Error::WEB_CRYPTO), }; - return Ok(RngSource::Browser(crypto)); + + let buf = Uint8Array::new_with_length(BROWSER_CRYPTO_BUFFER_SIZE as u32); + + let ctx = BrowserCryptoContext { + crypto, + buf, + }; + + return Ok(RngSource::Browser(ctx)); } let crypto = MODULE.require("crypto").map_err(|_| Error::NODE_CRYPTO)?; @@ -84,7 +97,7 @@ extern "C" { type BrowserCrypto; #[wasm_bindgen(method, js_name = getRandomValues, catch)] - fn get_random_values(me: &BrowserCrypto, buf: &mut [u8]) -> Result<(), JsValue>; + fn get_random_values(me: &BrowserCrypto, buf: &Uint8Array) -> Result<(), JsValue>; #[wasm_bindgen(js_name = module)] static MODULE: NodeModule; From 2831eccbad032444a94abd674f454bfbaedd7b56 Mon Sep 17 00:00:00 2001 From: chemicstry Date: Wed, 7 Oct 2020 00:58:08 +0300 Subject: [PATCH 2/5] Fix buffer length problem --- src/wasm-bindgen.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/wasm-bindgen.rs b/src/wasm-bindgen.rs index 0f33843cd..115bc1246 100644 --- a/src/wasm-bindgen.rs +++ b/src/wasm-bindgen.rs @@ -10,8 +10,8 @@ use crate::Error; extern crate std; use std::thread_local; -use wasm_bindgen::prelude::*; use js_sys::Uint8Array; +use wasm_bindgen::prelude::*; // Maximum is 65536 bytes see https://developer.mozilla.org/en-US/docs/Web/API/Crypto/getRandomValues const BROWSER_CRYPTO_BUFFER_SIZE: usize = 256; @@ -45,10 +45,14 @@ pub(crate) fn getrandom_inner(dest: &mut [u8]) -> Result<(), Error> { } RngSource::Browser(ctx) => { for chunk in dest.chunks_mut(BROWSER_CRYPTO_BUFFER_SIZE) { - if ctx.crypto.get_random_values(&ctx.buf).is_err() { + // chunk can be smaller than buf length + // this creates a smaller view to the buf memory without allocation + let sub_buf = ctx.buf.subarray(0, chunk.len() as u32); + + if ctx.crypto.get_random_values(&sub_buf).is_err() { return Err(Error::WEB_GET_RANDOM_VALUES); } - ctx.buf.copy_to(chunk); + sub_buf.copy_to(chunk); } } }; @@ -71,10 +75,7 @@ fn getrandom_init() -> Result { let buf = Uint8Array::new_with_length(BROWSER_CRYPTO_BUFFER_SIZE as u32); - let ctx = BrowserCryptoContext { - crypto, - buf, - }; + let ctx = BrowserCryptoContext { crypto, buf }; return Ok(RngSource::Browser(ctx)); } From abcdb2d6f09e44f63fa549a99fd95f5521021392 Mon Sep 17 00:00:00 2001 From: chemicstry Date: Fri, 23 Oct 2020 14:49:40 +0300 Subject: [PATCH 3/5] Simplify code --- src/wasm-bindgen.rs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/wasm-bindgen.rs b/src/wasm-bindgen.rs index 115bc1246..7a7dce236 100644 --- a/src/wasm-bindgen.rs +++ b/src/wasm-bindgen.rs @@ -16,15 +16,9 @@ use wasm_bindgen::prelude::*; // Maximum is 65536 bytes see https://developer.mozilla.org/en-US/docs/Web/API/Crypto/getRandomValues const BROWSER_CRYPTO_BUFFER_SIZE: usize = 256; -struct BrowserCryptoContext { - crypto: BrowserCrypto, - // A temporary buffer backed by browser memory to avoid multithreaded wasm exception. See issue #164 - buf: Uint8Array, -} - enum RngSource { Node(NodeCrypto), - Browser(BrowserCryptoContext), + Browser(BrowserCrypto, Uint8Array), } // JsValues are always per-thread, so we initialize RngSource for each thread. @@ -43,15 +37,18 @@ pub(crate) fn getrandom_inner(dest: &mut [u8]) -> Result<(), Error> { return Err(Error::NODE_RANDOM_FILL_SYNC); } } - RngSource::Browser(ctx) => { + RngSource::Browser(crypto, buf) => { + // getRandomValues does not work with all types of WASM memory, so + // we initially write to browser memory (buf) to avoid an exception. for chunk in dest.chunks_mut(BROWSER_CRYPTO_BUFFER_SIZE) { // chunk can be smaller than buf length // this creates a smaller view to the buf memory without allocation - let sub_buf = ctx.buf.subarray(0, chunk.len() as u32); + let sub_buf = buf.subarray(0, chunk.len() as u32); - if ctx.crypto.get_random_values(&sub_buf).is_err() { + if crypto.get_random_values(&sub_buf).is_err() { return Err(Error::WEB_GET_RANDOM_VALUES); } + sub_buf.copy_to(chunk); } } @@ -75,9 +72,7 @@ fn getrandom_init() -> Result { let buf = Uint8Array::new_with_length(BROWSER_CRYPTO_BUFFER_SIZE as u32); - let ctx = BrowserCryptoContext { crypto, buf }; - - return Ok(RngSource::Browser(ctx)); + return Ok(RngSource::Browser(crypto, buf)); } let crypto = MODULE.require("crypto").map_err(|_| Error::NODE_CRYPTO)?; From 4219cf0fa3ff8afe55bd712d704a6c09d3d352c4 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Mon, 26 Oct 2020 03:37:41 -0700 Subject: [PATCH 4/5] Whitespace nits Signed-off-by: Joe Richey --- src/wasm-bindgen.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/wasm-bindgen.rs b/src/wasm-bindgen.rs index 7a7dce236..d48c7d3a9 100644 --- a/src/wasm-bindgen.rs +++ b/src/wasm-bindgen.rs @@ -38,17 +38,16 @@ pub(crate) fn getrandom_inner(dest: &mut [u8]) -> Result<(), Error> { } } RngSource::Browser(crypto, buf) => { - // getRandomValues does not work with all types of WASM memory, so - // we initially write to browser memory (buf) to avoid an exception. + // getRandomValues does not work with all types of WASM memory, + // so we initially write to browser memory to avoid exceptions. for chunk in dest.chunks_mut(BROWSER_CRYPTO_BUFFER_SIZE) { - // chunk can be smaller than buf length - // this creates a smaller view to the buf memory without allocation + // The chunk can be smaller than buf's length, so we call to + // JS to create a smaller view of buf without allocation. let sub_buf = buf.subarray(0, chunk.len() as u32); if crypto.get_random_values(&sub_buf).is_err() { return Err(Error::WEB_GET_RANDOM_VALUES); } - sub_buf.copy_to(chunk); } } @@ -71,7 +70,6 @@ fn getrandom_init() -> Result { }; let buf = Uint8Array::new_with_length(BROWSER_CRYPTO_BUFFER_SIZE as u32); - return Ok(RngSource::Browser(crypto, buf)); } From 1ee238c986309f6d4aa695a850243d61fa8cfecf Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Mon, 26 Oct 2020 04:29:39 -0700 Subject: [PATCH 5/5] Try to fix CI Signed-off-by: Joe Richey --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index c931e477f..376550819 100644 --- a/.travis.yml +++ b/.travis.yml @@ -102,7 +102,7 @@ jobs: rust: nightly install: - rustup target add wasm32-unknown-unknown - - cargo --list | egrep "^\s*deadlinks$" -q || cargo install cargo-deadlinks + - cargo install cargo-deadlinks - cargo deadlinks -V script: # Check that setting various features does not break the build