Skip to content

Commit

Permalink
fix: don't leak canceled timeouts (#2331)
Browse files Browse the repository at this point in the history
Instead of using `Closure::once_into_js`, this uses `into_js_value`,
which uses weak references to clean up the closure when Javascript no
longer has need of it.

It would be nice to make this (and the similar interval function) drop
the callback promptly when cancelled, but I don't think that's
possible while keeping the handles Copy.

Fixes #2330

Co-authored-by: Robert Macomber <robertm@mox>
  • Loading branch information
rjmac and Robert Macomber authored Feb 20, 2024
1 parent aa3700f commit 47abe00
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions leptos_dom/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,25 @@ pub fn request_animation_frame(cb: impl FnOnce() + 'static) {
_ = request_animation_frame_with_handle(cb);
}

// Closure::once_into_js only frees the callback when it's actually
// called, so this instead uses into_js_value, which can be freed by
// the host JS engine's GC if it supports weak references (which all
// modern brower engines do). The way this works is that the provided
// callback's captured data is dropped immediately after being called,
// as before, but it leaves behind a small stub closure rust-side that
// will be freed "eventually" by the JS GC. If the function is never
// called (e.g., it's a cancelled timeout or animation frame callback)
// then it will also be freed eventually.
fn closure_once(cb: impl FnOnce() + 'static) -> JsValue {
let mut wrapped_cb: Option<Box<dyn FnOnce()>> = Some(Box::new(cb));
let closure = Closure::new(move || {
if let Some(cb) = wrapped_cb.take() {
cb()
}
});
closure.into_js_value()
}

/// Runs the given function between the next repaint using
/// [`Window.requestAnimationFrame`](https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame),
/// returning a cancelable handle.
Expand All @@ -128,7 +147,7 @@ pub fn request_animation_frame_with_handle(
.map(AnimationFrameRequestHandle)
}

raf(Closure::once_into_js(cb))
raf(closure_once(cb))
}

/// Handle that is generated by [request_idle_callback_with_handle] and can be
Expand Down Expand Up @@ -237,7 +256,7 @@ pub fn set_timeout_with_handle(
.map(TimeoutHandle)
}

st(Closure::once_into_js(cb), duration)
st(closure_once(cb), duration)
}

/// "Debounce" a callback function. This will cause it to wait for a period of `delay`
Expand Down

0 comments on commit 47abe00

Please sign in to comment.