Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,44 @@ 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
Comment thread
Madoshakalaka marked this conversation as resolved.
Outdated

- 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.
Comment thread
Madoshakalaka marked this conversation as resolved.
Outdated
- 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

test-history-wasi:
name: Test gloo-history WASI
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion crates/timers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ rust-version = { workspace = true }
features = ["futures"]

[dependencies]
wasm-bindgen = "0.2"
wasm-bindgen = "0.2.117"
Comment thread
Madoshakalaka marked this conversation as resolved.
Outdated
js-sys = { workspace = true }
futures-core = { version = "0.3", optional = true }
futures-channel = { version = "0.3", optional = true }
Expand Down
49 changes: 47 additions & 2 deletions crates/timers/src/callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: std::panic::UnwindSafe> 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.
Comment thread
guybedford marked this conversation as resolved.
Outdated
#[cfg(not(all(target_arch = "wasm32", panic = "unwind")))]
pub trait CallbackUnwindSafe {}
#[cfg(not(all(target_arch = "wasm32", panic = "unwind")))]
impl<T> CallbackUnwindSafe for T {}

#[wasm_bindgen]
unsafe extern "C" {
#[wasm_bindgen(js_name = "setTimeout", catch)]
Expand Down Expand Up @@ -57,8 +83,18 @@ impl Timeout {
/// ```
pub fn new<F>(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(
Expand Down Expand Up @@ -158,8 +194,17 @@ impl Interval {
/// ```
pub fn new<F>(millis: u32, callback: F) -> Interval
where
F: 'static + FnMut(),
F: 'static + FnMut() + CallbackUnwindSafe,
{
// Same rationale as `Timeout::new`: the `Box<F> as Box<dyn FnMut()>`
// 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<dyn FnMut()>);
#[cfg(not(all(target_arch = "wasm32", panic = "unwind")))]
let closure = Closure::wrap(Box::new(callback) as Box<dyn FnMut()>);

let id = set_interval(
Expand Down
40 changes: 39 additions & 1 deletion crates/timers/src/future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>` 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<T>` wrapper,
/// silently losing the unwind-safety assertion. Routing through a function
/// call forces capture of the wrapper.
#[inline(always)]
fn unwrap_assert_unwind_safe<T>(w: AssertUnwindSafe<T>) -> T {
w.0
}

/// A scheduled timeout as a `Future`.
///
/// See `TimeoutFuture::new` for scheduling new timeouts.
Expand Down Expand Up @@ -66,9 +78,20 @@ impl TimeoutFuture {
/// ```
pub fn new(millis: u32) -> TimeoutFuture {
let (tx, rx) = oneshot::channel();
// `oneshot::Sender` holds an `Arc<Inner<T>>` 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<Sender<()>>` (UnwindSafe) and not the inner
// `Sender<()>` (!UnwindSafe). See the discussion in PR #563.
Comment thread
guybedford marked this conversation as resolved.
Outdated
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 }
}
Expand Down Expand Up @@ -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<Inner<T>>` 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<UnboundedSender<()>>` 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();
Expand Down
Loading