diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2fcff04c..e97497b6 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -119,6 +119,34 @@ jobs: wasm-pack test --node crates/$x --no-default-features done + panic_unwind_build: + name: Build with -Cpanic=unwind + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - uses: dtolnay/rust-toolchain@master + with: + toolchain: nightly + target: wasm32-unknown-unknown + components: rust-src + + - uses: actions/cache@v4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: cargo-${{ runner.os }}-panic-unwind-${{ hashFiles('**/Cargo.toml') }} + restore-keys: | + cargo-${{ runner.os }}-panic-unwind- + cargo-${{ runner.os }}- + + - name: Build crates with panic=unwind + env: + RUSTFLAGS: '-Cpanic=unwind' + run: cargo build -p gloo-timers -p gloo-events -p gloo-render -p gloo-net -p gloo-worker --all-features --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind + test-history-wasi: name: Test gloo-history WASI runs-on: ubuntu-latest diff --git a/crates/events/src/lib.rs b/crates/events/src/lib.rs index cd5067c7..40f674cc 100644 --- a/crates/events/src/lib.rs +++ b/crates/events/src/lib.rs @@ -15,6 +15,24 @@ use wasm_bindgen::closure::Closure; use wasm_bindgen::{JsCast, UnwrapThrowExt}; use web_sys::{AddEventListenerOptions, Event, EventTarget}; +/// Bound carrying the unwind-safety requirement on [`EventListener`] callbacks. +/// +/// Under `panic = "unwind"` on wasm the callback is invoked across a +/// `catch_unwind` boundary inside `wasm_bindgen`, so this resolves to +/// [`std::panic::UnwindSafe`]. Under any other panic strategy it is a no-op +/// blanket. Wrap non-`UnwindSafe` captures in [`std::panic::AssertUnwindSafe`] +/// at the call site. +#[cfg(all(target_arch = "wasm32", panic = "unwind"))] +pub trait CallbackUnwindSafe: std::panic::UnwindSafe {} +#[cfg(all(target_arch = "wasm32", panic = "unwind"))] +impl CallbackUnwindSafe for T {} + +#[doc(hidden)] +#[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] +pub trait CallbackUnwindSafe {} +#[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] +impl CallbackUnwindSafe for T {} + /// Specifies whether the event listener is run during the capture or bubble phase. /// /// The official specification has [a good explanation](https://www.w3.org/TR/DOM-Level-3-Events/#event-flow) @@ -331,9 +349,18 @@ impl EventListener { pub fn new(target: &EventTarget, event_type: S, callback: F) -> Self where S: Into>, - F: FnMut(&Event) + 'static, + F: FnMut(&Event) + 'static + CallbackUnwindSafe, { - let callback = Closure::wrap(Box::new(callback) as Box); + // The `Box as Box` coercion erases the static + // `UnwindSafe` bound that wasm-bindgen 0.2.117+ requires under + // `panic = "unwind"`. The `CallbackUnwindSafe` bound on `F` has + // enforced unwind safety at the call site, so the internal + // `_assert_unwind_safe` is sound. + let inner = Box::new(callback) as Box; + #[cfg(all(target_arch = "wasm32", panic = "unwind"))] + let callback = Closure::wrap_assert_unwind_safe(inner); + #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] + let callback = Closure::wrap(inner); NEW_OPTIONS.with(move |options| { Self::raw_new( @@ -371,8 +398,13 @@ impl EventListener { pub fn once(target: &EventTarget, event_type: S, callback: F) -> Self where S: Into>, - F: FnOnce(&Event) + 'static, + F: FnOnce(&Event) + 'static + CallbackUnwindSafe, { + // See `EventListener::new` for the rationale on the cfg-gated + // `_assert_unwind_safe` variant. + #[cfg(all(target_arch = "wasm32", panic = "unwind"))] + let callback = Closure::once_assert_unwind_safe(callback); + #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] let callback = Closure::once(callback); ONCE_OPTIONS.with(move |options| { @@ -450,9 +482,14 @@ impl EventListener { ) -> Self where S: Into>, - F: FnMut(&Event) + 'static, + F: FnMut(&Event) + 'static + CallbackUnwindSafe, { - let callback = Closure::wrap(Box::new(callback) as Box); + // See `EventListener::new` for the rationale. + let inner = Box::new(callback) as Box; + #[cfg(all(target_arch = "wasm32", panic = "unwind"))] + let callback = Closure::wrap_assert_unwind_safe(inner); + #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] + let callback = Closure::wrap(inner); Self::raw_new( target, @@ -514,8 +551,12 @@ impl EventListener { ) -> Self where S: Into>, - F: FnOnce(&Event) + 'static, + F: FnOnce(&Event) + 'static + CallbackUnwindSafe, { + // See `EventListener::new` for the rationale. + #[cfg(all(target_arch = "wasm32", panic = "unwind"))] + let callback = Closure::once_assert_unwind_safe(callback); + #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] let callback = Closure::once(callback); Self::raw_new( diff --git a/crates/net/src/eventsource/futures.rs b/crates/net/src/eventsource/futures.rs index 89ff0e78..a77081c3 100644 --- a/crates/net/src/eventsource/futures.rs +++ b/crates/net/src/eventsource/futures.rs @@ -177,7 +177,7 @@ impl EventSource { let message_callback: Closure = { let event_type = event_type.clone(); let sender = message_sender.clone(); - Closure::wrap(Box::new(move |e: MessageEvent| { + wrap_internal!(Box::new(move |e: MessageEvent| { let event_type = event_type.clone(); let _ = sender.unbounded_send(StreamMessage::Message(event_type, e)); }) as Box) @@ -192,7 +192,7 @@ impl EventSource { .map_err(js_to_js_error)?; let error_callback: Closure = { - Closure::wrap(Box::new(move |e: web_sys::Event| { + wrap_internal!(Box::new(move |e: web_sys::Event| { let is_connecting = e .current_target() .map(|target| target.unchecked_into::()) diff --git a/crates/net/src/lib.rs b/crates/net/src/lib.rs index 25ce2cc5..c8c1a858 100644 --- a/crates/net/src/lib.rs +++ b/crates/net/src/lib.rs @@ -82,6 +82,29 @@ )] #![cfg_attr(docsrs, feature(doc_cfg))] +// wasm-bindgen 0.2.117+ requires `MaybeUnwindSafe` on closure inputs under +// `panic = "unwind"`. The `Box::new(...) as Box` coercion used to +// construct internal event callbacks erases any static `UnwindSafe` bound, +// so we route through `Closure::wrap_assert_unwind_safe` instead. The +// assertion is sound: every callback in this crate only forwards to ops on +// state we exclusively own — lock-free `mpsc::UnboundedSender` pushes and +// single-shot `Option` takes/wakes — so a panic across the +// `catch_unwind` boundary leaves observers seeing only states that are +// legitimately reachable (a missing wake, a missing message). On any other +// panic strategy this reduces to plain `Closure::wrap`. +#[cfg(all(target_arch = "wasm32", panic = "unwind"))] +macro_rules! wrap_internal { + ($e:expr) => { + wasm_bindgen::closure::Closure::wrap_assert_unwind_safe($e) + }; +} +#[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] +macro_rules! wrap_internal { + ($e:expr) => { + wasm_bindgen::closure::Closure::wrap($e) + }; +} + mod error; #[cfg(feature = "eventsource")] #[cfg_attr(docsrs, doc(cfg(feature = "eventsource")))] diff --git a/crates/net/src/websocket/futures.rs b/crates/net/src/websocket/futures.rs index 34d30b71..e20f7c01 100644 --- a/crates/net/src/websocket/futures.rs +++ b/crates/net/src/websocket/futures.rs @@ -133,7 +133,7 @@ impl WebSocket { let open_callback: Closure = { let waker = Rc::clone(&waker); - Closure::wrap(Box::new(move || { + wrap_internal!(Box::new(move || { if let Some(waker) = waker.borrow_mut().take() { waker.wake(); } @@ -151,7 +151,7 @@ impl WebSocket { let message_callback: Closure = { let sender = sender.clone(); - Closure::wrap(Box::new(move |e: MessageEvent| { + wrap_internal!(Box::new(move |e: MessageEvent| { let msg = parse_message(e); let _ = sender.unbounded_send(StreamMessage::Message(msg)); }) as Box) @@ -163,7 +163,7 @@ impl WebSocket { let error_callback: Closure = { let sender = sender.clone(); let waker = Rc::clone(&waker); - Closure::wrap(Box::new(move |_e: web_sys::Event| { + wrap_internal!(Box::new(move |_e: web_sys::Event| { if let Some(waker) = waker.borrow_mut().take() { waker.wake(); } @@ -175,7 +175,7 @@ impl WebSocket { .map_err(js_to_js_error)?; let close_callback: Closure = { - Closure::wrap(Box::new(move |e: web_sys::CloseEvent| { + wrap_internal!(Box::new(move |e: web_sys::CloseEvent| { let close_event = CloseEvent { code: e.code(), reason: e.reason(), diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 4f0c1e2a..febd2650 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -9,6 +9,25 @@ use std::rc::Rc; use wasm_bindgen::JsCast; use wasm_bindgen::prelude::*; +/// Bound carrying the unwind-safety requirement on [`request_animation_frame`] +/// callbacks. +/// +/// Under `panic = "unwind"` on wasm the callback is invoked across a +/// `catch_unwind` boundary inside `wasm_bindgen`, so this resolves to +/// [`std::panic::UnwindSafe`]. Under any other panic strategy it is a no-op +/// blanket. Wrap non-`UnwindSafe` captures in [`std::panic::AssertUnwindSafe`] +/// at the call site. +#[cfg(all(target_arch = "wasm32", panic = "unwind"))] +pub trait CallbackUnwindSafe: std::panic::UnwindSafe {} +#[cfg(all(target_arch = "wasm32", panic = "unwind"))] +impl CallbackUnwindSafe for T {} + +#[doc(hidden)] +#[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] +pub trait CallbackUnwindSafe {} +#[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] +impl CallbackUnwindSafe for T {} + /// Handle for [`request_animation_frame`]. #[derive(Debug)] pub struct AnimationFrame { @@ -40,16 +59,31 @@ impl Drop for AnimationFrame { /// [MDN Documentation](https://developer.mozilla.org/en-US/docs/Web/API/Window/requestAnimationFrame) pub fn request_animation_frame(callback_once: F) -> AnimationFrame where - F: FnOnce(f64) + 'static, + F: FnOnce(f64) + 'static + CallbackUnwindSafe, { let callback_wrapper = Rc::new(RefCell::new(Some(CallbackWrapper(Box::new(callback_once))))); + // The internal trampoline captures `Rc>>` + // which is not `UnwindSafe`. Asserting unwind safety here is sound: the + // `borrow_mut().take()` releases the borrow before invoking the user + // closure, and a panic inside the user closure leaves the cell holding + // `None` — a valid post-fire state. `Drop` checks `is_some()` and skips + // cancellation when the slot is already empty. The `CallbackUnwindSafe` + // bound on the public API enforces unwind safety at the call site. let callback: Closure = { let callback_wrapper = Rc::clone(&callback_wrapper); - Closure::wrap(Box::new(move |v: JsValue| { + let inner = Box::new(move |v: JsValue| { let time: f64 = v.as_f64().unwrap_or(0.0); let callback = callback_wrapper.borrow_mut().take().unwrap().0; callback(time); - })) + }) as Box; + #[cfg(all(target_arch = "wasm32", panic = "unwind"))] + { + Closure::wrap_assert_unwind_safe(inner) + } + #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] + { + Closure::wrap(inner) + } }; let render_id = web_sys::window() diff --git a/crates/timers/src/callback.rs b/crates/timers/src/callback.rs index 68dc5ee7..a7f441f2 100644 --- a/crates/timers/src/callback.rs +++ b/crates/timers/src/callback.rs @@ -4,6 +4,25 @@ use js_sys::Function; use wasm_bindgen::prelude::*; use wasm_bindgen::{JsCast, JsValue}; +/// Bound carrying the unwind-safety requirement on [`Timeout::new`] / +/// [`Interval::new`] callbacks. +/// +/// Under `panic = "unwind"` on wasm the callback is invoked across a +/// `catch_unwind` boundary inside `wasm_bindgen`, so this resolves to +/// [`std::panic::UnwindSafe`]. Under any other panic strategy it is a no-op +/// blanket. Wrap non-`UnwindSafe` captures in [`std::panic::AssertUnwindSafe`] +/// at the call site. +#[cfg(all(target_arch = "wasm32", panic = "unwind"))] +pub trait CallbackUnwindSafe: std::panic::UnwindSafe {} +#[cfg(all(target_arch = "wasm32", panic = "unwind"))] +impl CallbackUnwindSafe for T {} + +#[doc(hidden)] +#[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] +pub trait CallbackUnwindSafe {} +#[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] +impl CallbackUnwindSafe for T {} + #[wasm_bindgen] unsafe extern "C" { #[wasm_bindgen(js_name = "setTimeout", catch)] @@ -57,8 +76,18 @@ impl Timeout { /// ``` pub fn new(millis: u32, callback: F) -> Timeout where - F: 'static + FnOnce(), + F: 'static + FnOnce() + CallbackUnwindSafe, { + // Under `panic = "unwind"` we use the `_assert_unwind_safe` variant + // because `WasmClosureFnOnce` selection erases `F` into a trait-object + // dispatch that no longer carries the static `UnwindSafe` bound — the + // same dyn-erasure problem wasm-bindgen handles internally. The + // `CallbackUnwindSafe` bound on the public API has already enforced + // the requirement at the call site. On any other panic strategy + // `Closure::once` is unchanged and works on any 0.2.x wasm-bindgen. + #[cfg(all(target_arch = "wasm32", panic = "unwind"))] + let closure = Closure::once_assert_unwind_safe(callback); + #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] let closure = Closure::once(callback); let id = set_timeout( @@ -158,8 +187,17 @@ impl Interval { /// ``` pub fn new(millis: u32, callback: F) -> Interval where - F: 'static + FnMut(), + F: 'static + FnMut() + CallbackUnwindSafe, { + // Same rationale as `Timeout::new`: the `Box as Box` + // coercion erases the `UnwindSafe` bound, so under `panic = "unwind"` + // we use `_assert_unwind_safe` to acknowledge the erasure (the public + // `CallbackUnwindSafe` bound has already enforced unwind safety at + // the call site). Otherwise the original `Closure::wrap` path is + // preserved for older wasm-bindgen compatibility. + #[cfg(all(target_arch = "wasm32", panic = "unwind"))] + let closure = Closure::wrap_assert_unwind_safe(Box::new(callback) as Box); + #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] let closure = Closure::wrap(Box::new(callback) as Box); let id = set_interval( diff --git a/crates/timers/src/future.rs b/crates/timers/src/future.rs index 398d4ce9..b324ee81 100644 --- a/crates/timers/src/future.rs +++ b/crates/timers/src/future.rs @@ -5,11 +5,23 @@ use crate::callback::{Interval, Timeout}; use futures_channel::{mpsc, oneshot}; use futures_core::stream::Stream; use std::convert::TryFrom; +use std::panic::AssertUnwindSafe; use std::pin::Pin; use std::task::{Context, Poll}; use std::time::Duration; use wasm_bindgen::prelude::*; +/// Unwrap an `AssertUnwindSafe` by-value without triggering RFC 2229 +/// field-level disjoint capture inside a closure. Writing `w.0` or +/// `let AssertUnwindSafe(x) = w` inside a `move` closure causes the closure +/// to capture the inner `T` instead of the `AssertUnwindSafe` wrapper, +/// silently losing the unwind-safety assertion. Routing through a function +/// call forces capture of the wrapper. +#[inline(always)] +fn unwrap_assert_unwind_safe(w: AssertUnwindSafe) -> T { + w.0 +} + /// A scheduled timeout as a `Future`. /// /// See `TimeoutFuture::new` for scheduling new timeouts. @@ -66,9 +78,20 @@ impl TimeoutFuture { /// ``` pub fn new(millis: u32) -> TimeoutFuture { let (tx, rx) = oneshot::channel(); + // `oneshot::Sender` holds an `Arc>` whose interior is an + // `UnsafeCell`-backed lock, so it is not `UnwindSafe`. The assertion + // is sound: the closure is `FnOnce`, `tx` is consumed by `send`, and + // nothing observes the sender after the callback returns. `rx` takes + // the same lock on poll and treats a missing payload as "cancelled". + // + // The wrapper is unwound by a helper rather than `let _(x) = w;` or + // `w.0` so that RFC 2229 disjoint-capture sees the closure capturing + // `AssertUnwindSafe>` (UnwindSafe) and not the inner + // `Sender<()>` (!UnwindSafe). See the discussion in PR #562. + let tx = AssertUnwindSafe(tx); let inner = Timeout::new(millis, move || { // if the receiver was dropped we do nothing. - tx.send(()).unwrap_throw(); + unwrap_assert_unwind_safe(tx).send(()).unwrap_throw(); }); TimeoutFuture { _inner: inner, rx } } @@ -140,6 +163,21 @@ impl IntervalStream { /// ``` pub fn new(millis: u32) -> IntervalStream { let (sender, receiver) = mpsc::unbounded(); + // `mpsc::UnboundedSender` shares state with the receiver through an + // `Arc>` with `UnsafeCell` interior, so it is not + // `UnwindSafe`. The assertion is sound: `unbounded_send` is a + // lock-free push that either completes or doesn't, and the only + // realistic panic site (allocation) aborts under default config; if + // a future ticks observes an inconsistent queue it can at worst + // hang, not violate memory safety. + // + // `unbounded_send` takes `&self`, so the method call autoderefs + // through `AssertUnwindSafe`'s `Deref` impl and the closure captures + // `AssertUnwindSafe>` rather than projecting to + // the inner `Sender`. Avoid rewriting as `sender.0.unbounded_send(...)` + // — that explicit `.0` defeats the wrapper under RFC 2229 + // disjoint-capture inference. + let sender = AssertUnwindSafe(sender); let inner = Interval::new(millis, move || { // if the receiver was dropped we do nothing. sender.unbounded_send(()).unwrap_throw(); diff --git a/crates/worker/src/actor/native_worker.rs b/crates/worker/src/actor/native_worker.rs index e7c8ac7f..9139f0fb 100644 --- a/crates/worker/src/actor/native_worker.rs +++ b/crates/worker/src/actor/native_worker.rs @@ -46,7 +46,20 @@ macro_rules! worker_ext_impl { let msg = CODEC::decode(message.data()); handler(msg); }; - let closure = Closure::wrap(Box::new(handler) as Box).into_js_value(); + // wasm-bindgen 0.2.117+ requires `MaybeUnwindSafe` on closure + // inputs under `panic = "unwind"`; the `Box as Box` + // coercion erases that bound. The internal callers (spawner, + // registrar) supply handlers that touch `Rc>` of + // worker state via `borrow_mut().take()` patterns that release + // borrows before invoking nested user code, so a panic across + // the `catch_unwind` boundary leaves no observable invariant + // violation. Asserting unwind safety here is sound. On other + // panic strategies this is plain `Closure::wrap`. + let inner = Box::new(handler) as Box; + #[cfg(all(target_arch = "wasm32", panic = "unwind"))] + let closure = Closure::wrap_assert_unwind_safe(inner).into_js_value(); + #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] + let closure = Closure::wrap(inner).into_js_value(); self.set_onmessage(Some(closure.as_ref().unchecked_ref())); }