From 82b17dad7a00327839157476bc355cd7ea542d56 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 27 Apr 2026 17:19:54 -0700 Subject: [PATCH 1/7] fix(timers): propagate unwind safety to user callbacks wasm-bindgen 0.2.117+ added `MaybeUnwindSafe` bounds on `Closure::wrap` and `Closure::once`, because closures handed to JS are invoked across a `catch_unwind` boundary under `panic = "unwind"`. `gloo-timers` accepts arbitrary user `FnOnce` / `FnMut` callbacks, but its `Closure::wrap(...)` / `Closure::once(...)` calls fail to compile under panic=unwind because the `Box::new(callback) as Box` coercion (and the `FnOnce` trait selection) erase the static `UnwindSafe` bound that wasm-bindgen requires. This breaks every downstream crate that pulls in `gloo-timers` as a `dev-dependency` and tests under `-Cpanic=unwind` \u2014 see the failure on https://github.com/MattiasBuelens/wasm-streams/pull/35. Fix: - Surface the requirement at the public API: `Timeout::new` / `Interval::new` now require `F: CallbackUnwindSafe`, a marker that resolves to `std::panic::UnwindSafe` under `panic = "unwind"` on wasm and to a no-op blanket otherwise. Callers with non-`UnwindSafe` captures must wrap them in `std::panic::AssertUnwindSafe` at the call site, which is where the invariants can actually be reasoned about. - Internally use `Closure::wrap_assert_unwind_safe` / `Closure::once_assert_unwind_safe` under panic=unwind to acknowledge the dyn-erasure explicitly. The public bound has already enforced the requirement at the call site, so the internal assertion is sound. - Bump the minimum `wasm-bindgen` requirement to `0.2.117` (where the `_assert_unwind_safe` helpers were added). - Add a `panic_unwind_build` CI job that builds `gloo-timers` with `-Cpanic=unwind` so this regression cannot recur silently. --- .github/workflows/tests.yml | 37 ++++++++++++++++++++++++++ crates/timers/Cargo.toml | 2 +- crates/timers/src/callback.rs | 49 +++++++++++++++++++++++++++++++++-- 3 files changed, 85 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2fcff04c..21650737 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -119,6 +119,43 @@ 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 + + # Recent Rust nightlies with LLVM 22 are temporarily broken with + # `panic=unwind` (see wasm-bindgen/wasm-bindgen#4929). Pin to a known-good + # nightly to avoid flapping. + - uses: dtolnay/rust-toolchain@master + with: + toolchain: nightly-2026-01-28 + 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 }}- + + # Verify gloo-timers (and any other crate that hosts user closures via + # `Closure::wrap` / `Closure::once`) still builds cleanly under + # `panic = "unwind"` against the new `MaybeUnwindSafe` bounds added in + # wasm-bindgen 0.2.117+. Catches downstream regressions like + # https://github.com/MattiasBuelens/wasm-streams/pull/35 from creeping in. + - name: Build crates with panic=unwind + env: + RUSTFLAGS: '-Cpanic=unwind' + run: | + cargo build -p gloo-timers --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/timers/Cargo.toml b/crates/timers/Cargo.toml index e9124a91..0f5874a8 100644 --- a/crates/timers/Cargo.toml +++ b/crates/timers/Cargo.toml @@ -15,7 +15,7 @@ rust-version = { workspace = true } features = ["futures"] [dependencies] -wasm-bindgen = "0.2" +wasm-bindgen = "0.2.117" js-sys = { workspace = true } futures-core = { version = "0.3", optional = true } futures-channel = { version = "0.3", optional = true } diff --git a/crates/timers/src/callback.rs b/crates/timers/src/callback.rs index 68dc5ee7..5ca13bd2 100644 --- a/crates/timers/src/callback.rs +++ b/crates/timers/src/callback.rs @@ -4,6 +4,32 @@ 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 {} + +/// 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(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 +83,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 +194,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( From d73276ed358882349d5edfb26c97573653dc389c Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 13 May 2026 17:47:13 -0700 Subject: [PATCH 2/7] fix(timers): apply unwind safety to futures feature The previous commit propagated `CallbackUnwindSafe` through `Timeout::new` / `Interval::new`, but the `futures` module's own closures capture `oneshot::Sender` and `mpsc::UnboundedSender`, both of which hold an `Arc>` with `UnsafeCell` interior and so are not `UnwindSafe`. Building `gloo-timers --features futures` under `-Cpanic=unwind` failed against the new bound; the CI job only exercised default features so the regression went undetected. Wrap each sender in `AssertUnwindSafe` and ensure the closure captures the wrapper rather than the inner sender. RFC 2229 disjoint capture projects through any explicit field access (`tx.0`) or irrefutable destructure (`let AssertUnwindSafe(x) = tx`), defeating the wrapper. The `TimeoutFuture` case (FnOnce, `send` consumes `self`) routes the unwrap through a tiny helper so the captured path stays at the wrapper. The `IntervalStream` case (FnMut, `unbounded_send` takes `&self`) already autoderefs through `AssertUnwindSafe: Deref`, so just wrapping at the bind site is sufficient. Soundness: for `TimeoutFuture` the sender is consumed and never observed again, so any panic inside `send` cannot expose torn state. For `IntervalStream` `unbounded_send` is a lock-free push whose only realistic panic site is allocation, which aborts under default config; the worst-case observable consequence of an interrupted push is a hung stream, not a memory-safety violation. Extend the panic=unwind CI job to also build with `--features futures` so this path is exercised. --- .github/workflows/tests.yml | 1 + crates/timers/src/future.rs | 40 ++++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 21650737..ac58fa8a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -155,6 +155,7 @@ jobs: RUSTFLAGS: '-Cpanic=unwind' run: | cargo build -p gloo-timers --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind + cargo build -p gloo-timers --features futures --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind test-history-wasi: name: Test gloo-history WASI diff --git a/crates/timers/src/future.rs b/crates/timers/src/future.rs index 398d4ce9..7259b297 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 #563. + 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(); From c2175a1f53fadcb38b65f58b5db03c3d5f285d1a Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 15 May 2026 13:57:05 -0700 Subject: [PATCH 3/7] fix(timers): address review feedback on unwind safety - Drop the wasm-bindgen floor bump back to 0.2; the _assert_unwind_safe paths are cfg-gated on panic = "unwind" so default builds remain compatible with any 0.2.x release. - Deduplicate the CallbackUnwindSafe doc block (the no-op blanket now carries #[doc(hidden)] instead of a copy of the marker docs). - Fix stale PR reference in the AssertUnwindSafe capture rationale. --- crates/timers/Cargo.toml | 2 +- crates/timers/src/callback.rs | 9 +-------- crates/timers/src/future.rs | 2 +- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/crates/timers/Cargo.toml b/crates/timers/Cargo.toml index 0f5874a8..e9124a91 100644 --- a/crates/timers/Cargo.toml +++ b/crates/timers/Cargo.toml @@ -15,7 +15,7 @@ rust-version = { workspace = true } features = ["futures"] [dependencies] -wasm-bindgen = "0.2.117" +wasm-bindgen = "0.2" js-sys = { workspace = true } futures-core = { version = "0.3", optional = true } futures-channel = { version = "0.3", optional = true } diff --git a/crates/timers/src/callback.rs b/crates/timers/src/callback.rs index 5ca13bd2..a7f441f2 100644 --- a/crates/timers/src/callback.rs +++ b/crates/timers/src/callback.rs @@ -17,14 +17,7 @@ pub trait CallbackUnwindSafe: std::panic::UnwindSafe {} #[cfg(all(target_arch = "wasm32", panic = "unwind"))] impl CallbackUnwindSafe for T {} -/// 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. +#[doc(hidden)] #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] pub trait CallbackUnwindSafe {} #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] diff --git a/crates/timers/src/future.rs b/crates/timers/src/future.rs index 7259b297..b324ee81 100644 --- a/crates/timers/src/future.rs +++ b/crates/timers/src/future.rs @@ -87,7 +87,7 @@ impl TimeoutFuture { // 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 #563. + // `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. From 0ad3861024a364c2dca5e0fb750edc29f16b9d28 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 15 May 2026 13:57:20 -0700 Subject: [PATCH 4/7] fix(events,render,net,worker): propagate unwind safety to user callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply the same fix as gloo-timers to every sibling crate that hosts wasm-bindgen `Closure::wrap` / `Closure::once` calls. Without this, each of these crates fails to build under `-Cpanic=unwind` against wasm-bindgen 0.2.117+ because the `Box as Box` coercion strips the static `MaybeUnwindSafe` bound the closure constructors now require. - gloo-events: surface `CallbackUnwindSafe` on the four public `EventListener` constructors and route through `Closure::wrap_assert_unwind_safe` / `once_assert_unwind_safe` under panic=unwind. - gloo-render: same on `request_animation_frame`. The internal trampoline captures `Rc>>` and is audited unwind-safe (borrow released before invoking the user closure; Drop sees a valid post-fire `None` state on panic). - gloo-net: internal-only closures in the websocket and eventsource futures. No public bound needed — every callback forwards to lock-free `mpsc::UnboundedSender` pushes and single-shot `Option` takes on state we exclusively own. A shared `wrap_internal!` macro at the crate root keeps the cfg branches in one place. - gloo-worker: internal `set_on_packed_message` trampoline; the spawner/registrar handlers release `RefCell` borrows before invoking nested user code so the assertion is sound. All under-panic=unwind code paths are cfg-gated, so default panic=abort builds are unchanged on any 0.2.x wasm-bindgen. --- crates/events/src/lib.rs | 53 +++++++++++++++++++++--- crates/net/src/eventsource/futures.rs | 4 +- crates/net/src/lib.rs | 15 +++++++ crates/net/src/websocket/futures.rs | 8 ++-- crates/render/src/lib.rs | 36 ++++++++++++++-- crates/worker/src/actor/native_worker.rs | 15 ++++++- 6 files changed, 115 insertions(+), 16 deletions(-) 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..7cc09aff 100644 --- a/crates/net/src/lib.rs +++ b/crates/net/src/lib.rs @@ -82,6 +82,21 @@ )] #![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..19a67527 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,27 @@ 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/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())); } From 17111d09bce7408c240103da79b972b8f61a7116 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 15 May 2026 13:57:29 -0700 Subject: [PATCH 5/7] ci: extend panic=unwind build coverage to all gloo crates Previously this CI job only built gloo-timers with default features, which let the futures-feature regression go undetected during review and would not catch the same regression in any sibling crate. Build every gloo crate that wraps user closures via wasm-bindgen under `-Cpanic=unwind` so any future change that breaks `MaybeUnwindSafe` compatibility fails CI loudly. Also drop the nightly-2026-01-28 pin: the upstream LLVM 22 + `panic=unwind` exception-handling-tag breakage (wasm-bindgen#4929) is fixed. --- .github/workflows/tests.yml | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ac58fa8a..4e701c30 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -125,12 +125,9 @@ jobs: steps: - uses: actions/checkout@v6 - # Recent Rust nightlies with LLVM 22 are temporarily broken with - # `panic=unwind` (see wasm-bindgen/wasm-bindgen#4929). Pin to a known-good - # nightly to avoid flapping. - uses: dtolnay/rust-toolchain@master with: - toolchain: nightly-2026-01-28 + toolchain: nightly target: wasm32-unknown-unknown components: rust-src @@ -145,17 +142,23 @@ jobs: cargo-${{ runner.os }}-panic-unwind- cargo-${{ runner.os }}- - # Verify gloo-timers (and any other crate that hosts user closures via - # `Closure::wrap` / `Closure::once`) still builds cleanly under - # `panic = "unwind"` against the new `MaybeUnwindSafe` bounds added in - # wasm-bindgen 0.2.117+. Catches downstream regressions like - # https://github.com/MattiasBuelens/wasm-streams/pull/35 from creeping in. + # Verify every gloo crate that wraps user closures via `Closure::wrap` + # / `Closure::once` still builds cleanly under `panic = "unwind"` + # against the `MaybeUnwindSafe` bounds added in wasm-bindgen 0.2.117+. + # Catches downstream regressions like + # https://github.com/MattiasBuelens/wasm-streams/pull/35 from creeping + # in. The crates listed here are the ones that host wasm-bindgen + # closures; the others have no relevant code path. - name: Build crates with panic=unwind env: RUSTFLAGS: '-Cpanic=unwind' run: | cargo build -p gloo-timers --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind cargo build -p gloo-timers --features futures --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind + cargo build -p gloo-events --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind + cargo build -p gloo-render --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind + cargo build -p gloo-net --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind + cargo build -p gloo-worker --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind test-history-wasi: name: Test gloo-history WASI From af0f977564a27fd92ce3a185d585ac0d6c4c2da4 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 15 May 2026 13:59:09 -0700 Subject: [PATCH 6/7] style: rustfmt the new macro and cfg blocks --- crates/net/src/lib.rs | 12 ++++++++++-- crates/render/src/lib.rs | 8 ++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/crates/net/src/lib.rs b/crates/net/src/lib.rs index 7cc09aff..c8c1a858 100644 --- a/crates/net/src/lib.rs +++ b/crates/net/src/lib.rs @@ -93,9 +93,17 @@ // 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) } } +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) } } +macro_rules! wrap_internal { + ($e:expr) => { + wasm_bindgen::closure::Closure::wrap($e) + }; +} mod error; #[cfg(feature = "eventsource")] diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 19a67527..febd2650 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -77,9 +77,13 @@ where callback(time); }) as Box; #[cfg(all(target_arch = "wasm32", panic = "unwind"))] - { Closure::wrap_assert_unwind_safe(inner) } + { + Closure::wrap_assert_unwind_safe(inner) + } #[cfg(not(all(target_arch = "wasm32", panic = "unwind")))] - { Closure::wrap(inner) } + { + Closure::wrap(inner) + } }; let render_id = web_sys::window() From e3def81d8cdffd6141763a759318ae1295d1b381 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 15 May 2026 14:03:46 -0700 Subject: [PATCH 7/7] ci: consolidate panic=unwind builds into a single cargo invocation --- .github/workflows/tests.yml | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4e701c30..e97497b6 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -142,23 +142,10 @@ jobs: cargo-${{ runner.os }}-panic-unwind- cargo-${{ runner.os }}- - # Verify every gloo crate that wraps user closures via `Closure::wrap` - # / `Closure::once` still builds cleanly under `panic = "unwind"` - # against the `MaybeUnwindSafe` bounds added in wasm-bindgen 0.2.117+. - # Catches downstream regressions like - # https://github.com/MattiasBuelens/wasm-streams/pull/35 from creeping - # in. The crates listed here are the ones that host wasm-bindgen - # closures; the others have no relevant code path. - name: Build crates with panic=unwind env: RUSTFLAGS: '-Cpanic=unwind' - run: | - cargo build -p gloo-timers --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind - cargo build -p gloo-timers --features futures --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind - cargo build -p gloo-events --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind - cargo build -p gloo-render --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind - cargo build -p gloo-net --target wasm32-unknown-unknown -Zbuild-std=std,panic_unwind - cargo build -p gloo-worker --target wasm32-unknown-unknown -Zbuild-std=std,panic_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