diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 83d0426..65bf559 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,7 +46,7 @@ jobs: run: cargo test --target=x86_64-unknown-linux-gnu - name: Install wasm-pack - run: curl https://drager.github.io/wasm-pack/installer/init.sh -sSf | bash + run: curl https://wasm-bindgen.github.io/wasm-pack/installer/init.sh -sSf | bash - name: Test on Node run: wasm-pack test --node -- ${{ matrix.unstable-flags }} diff --git a/Cargo.toml b/Cargo.toml index b704399..3adf012 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,7 +53,7 @@ features = [ wasm-bindgen-test = "0.3.58" tokio = { version = "^1", features = ["macros", "rt"] } pin-project = "^1" -gloo-timers = { version = "^0.3.0", features = ["futures"] } +gloo-timers = { version = "^0.4.0", features = ["futures"] } [dev-dependencies.web-sys] version = "^0.3.85" @@ -70,3 +70,7 @@ features = [ [package.metadata.docs.rs] # https://blog.rust-lang.org/2020/03/15/docs-rs-opt-into-fewer-targets.html targets = ["x86_64-unknown-linux-gnu"] + +# Pending gloo PR https://github.com/rustwasm/gloo/pull/562 with unwind safety +[patch.crates-io] +gloo-timers = { git = "https://github.com/guybedford/gloo", branch = "fix-gloo-timers-unwind-safety" } diff --git a/src/readable/into_underlying_byte_source.rs b/src/readable/into_underlying_byte_source.rs index 14c1e48..ea73f82 100644 --- a/src/readable/into_underlying_byte_source.rs +++ b/src/readable/into_underlying_byte_source.rs @@ -1,5 +1,5 @@ use std::cell::RefCell; -use std::panic::AssertUnwindSafe; +use std::panic::{AssertUnwindSafe, RefUnwindSafe, UnwindSafe}; use std::pin::Pin; use std::rc::Rc; @@ -21,6 +21,16 @@ pub(crate) struct IntoUnderlyingByteSource { pull_handle: Option, } +// SAFETY: `Inner` holds an `Option>>` and uses the +// take-and-replace pattern across every fallible await point in `Inner::pull`. +// On panic, the inner async_read is already taken out of the `Option`, leaving +// the cell in a clean `None` state. The `Rc>` interior mutability +// therefore cannot expose torn invariants to a subsequent call after a caught +// panic, satisfying the logical unwind-safety contract enforced by +// `#[wasm_bindgen]` exports under `panic = "unwind"`. +impl UnwindSafe for IntoUnderlyingByteSource {} +impl RefUnwindSafe for IntoUnderlyingByteSource {} + impl IntoUnderlyingByteSource { pub fn new(async_read: Box, default_buffer_len: usize) -> Self { IntoUnderlyingByteSource { diff --git a/src/readable/into_underlying_source.rs b/src/readable/into_underlying_source.rs index f4108b5..2e3f55c 100644 --- a/src/readable/into_underlying_source.rs +++ b/src/readable/into_underlying_source.rs @@ -1,5 +1,5 @@ use std::cell::RefCell; -use std::panic::AssertUnwindSafe; +use std::panic::{AssertUnwindSafe, RefUnwindSafe, UnwindSafe}; use std::pin::Pin; use std::rc::Rc; @@ -19,6 +19,15 @@ pub(crate) struct IntoUnderlyingSource { pull_handle: Option, } +// SAFETY: `Inner` holds an `Option>>` and uses the +// take-and-replace pattern around every fallible await in `Inner::pull`. On +// panic, the stream is already taken out of the `Option`, leaving the cell in +// a clean `None` state. Subsequent calls fail cleanly via `unwrap_throw` +// rather than observing a torn intermediate state, which upholds the logical +// unwind-safety contract `#[wasm_bindgen]` enforces for exported types. +impl UnwindSafe for IntoUnderlyingSource {} +impl RefUnwindSafe for IntoUnderlyingSource {} + impl IntoUnderlyingSource { pub fn new(stream: Box) -> Self { IntoUnderlyingSource { diff --git a/src/writable/into_underlying_sink.rs b/src/writable/into_underlying_sink.rs index aa9f35d..77d5b61 100644 --- a/src/writable/into_underlying_sink.rs +++ b/src/writable/into_underlying_sink.rs @@ -1,5 +1,5 @@ use std::cell::RefCell; -use std::panic::AssertUnwindSafe; +use std::panic::{AssertUnwindSafe, RefUnwindSafe, UnwindSafe}; use std::pin::Pin; use std::rc::Rc; @@ -13,6 +13,16 @@ pub(crate) struct IntoUnderlyingSink { inner: Rc>, } +// SAFETY: `Inner` holds an `Option>>>` and uses the +// take-and-replace pattern around every fallible await in `Inner::write` / +// `Inner::close` / `Inner::abort`. On panic the sink is already taken out of +// the `Option`, leaving the cell in a clean `None` state. The +// `Rc>` interior mutability therefore cannot expose torn +// invariants after a caught panic, upholding the logical unwind-safety +// contract enforced by `#[wasm_bindgen]` exports under `panic = "unwind"`. +impl UnwindSafe for IntoUnderlyingSink {} +impl RefUnwindSafe for IntoUnderlyingSink {} + impl IntoUnderlyingSink { pub fn new(sink: Box>) -> Self { IntoUnderlyingSink { diff --git a/tests/tests/fetch_as_stream.rs b/tests/tests/fetch_as_stream.rs index fd56f63..8789009 100644 --- a/tests/tests/fetch_as_stream.rs +++ b/tests/tests/fetch_as_stream.rs @@ -10,7 +10,7 @@ use web_sys::{Response, Window}; #[wasm_bindgen_test] async fn test_fetch_as_stream() { // Make a fetch request - let url = "https://drager.github.io/wasm-pack/public/img/wasm-ferris.png"; + let url = "https://wasm-bindgen.github.io/wasm-pack/public/img/wasm-ferris.png"; let window = global().unchecked_into::(); // hack to also support Node.js let resp_value = JsFuture::from(window.fetch_with_str(url)) .await diff --git a/tests/util/unhandled_error_guard.rs b/tests/util/unhandled_error_guard.rs index 3415715..db371e1 100644 --- a/tests/util/unhandled_error_guard.rs +++ b/tests/util/unhandled_error_guard.rs @@ -1,4 +1,5 @@ use std::cell::RefCell; +use std::panic::AssertUnwindSafe; use std::rc::Rc; use wasm_bindgen::closure::Closure; use wasm_bindgen::{JsCast, JsValue}; @@ -16,12 +17,15 @@ impl UnhandledErrorGuard { // Add a listener that collects any errors let errors = Rc::new(RefCell::new(vec![])); let listener = { - let errors = errors.clone(); + let errors = AssertUnwindSafe(errors.clone()); Closure::::new(move |event: JsValue| { - if let Some(event) = event.dyn_ref::() { - errors.borrow_mut().push(event.error()); - } else if let Some(event) = event.dyn_ref::() { - errors.borrow_mut().push(event.reason()); + let err = if let Some(event) = event.dyn_ref::() { + Some(event.error()) + } else { + event.dyn_ref::().map(|e| e.reason()) + }; + if let Some(err) = err { + errors.borrow_mut().push(err); } }) };