Skip to content

auto_update leaks the frame-loop Closure on every call (self-referential Rc cycle never broken) #349

@ademilsonfp

Description

@ademilsonfp

Summary

auto_update in packages/dom/src/auto_update.rs builds a self-rescheduling
animation-frame closure whose only strong reference is the Rc it captures of
itself. The cleanup function returned by auto_update cancels the pending
animation frame but never clears that cell, so the Rc cycle is never broken.
As a result every call to auto_update permanently leaks the closure together
with everything it captures — a clone of the reference element and the update
callback.

Affected crate

  • floating-ui-dom (observed on the current main, crate version 0.7.0;
    the same code is present in the published 0.6.0).
  • Reached transitively from floating-ui-leptos (and presumably the Yew/Dioxus
    bindings) through use_floating + while_elements_mounted / auto_update.

Location

packages/dom/src/auto_update.rs, inside pub fn auto_update.

Root cause

The frame-loop closure is stored in an Rc<RefCell<Option<Closure>>> and
captures a clone of that same Rc so it can reschedule itself:

let frame_loop_closure = Rc::new(RefCell::new(None));
let frame_loop_closure_clone = frame_loop_closure.clone();

*frame_loop_closure_clone.borrow_mut() = Some(Closure::new({
    // ...
    move || {
        // ...
        frame_loop_frame_id.replace(Some(request_animation_frame(
            frame_loop_closure            // <-- captures the Rc that owns this closure
                .borrow()
                .as_ref()
                .expect("Frame loop closure should exist."),
        )));
    }
}));

This forms a strong cycle: the Closure is owned by the RefCell behind the
Rc, and the Closure itself holds a clone of that Rc. After auto_update
returns, the locals frame_loop_closure / frame_loop_closure_clone go out of
scope and the only remaining strong reference is the closure's reference to
itself, so its strong count never reaches zero.

The cleanup closure returned by auto_update only cancels the scheduled frame —
it never sets the cell back to None:

Box::new(move || {
    // ... remove scroll/resize listeners, disconnect ResizeObserver, etc. ...

    if let Some(frame_id) = frame_id.take() {
        cancel_animation_frame(frame_id);
    }
    // frame_loop_closure is never cleared here -> cycle survives
})

Note the closure is constructed unconditionally, regardless of the
animation_frame option (the if animation_frame block only controls whether
it is scheduled). So the leak happens in both modes — auto_update with and
without animation_frame.

Impact

Every auto_update call leaks:

  • the frame-loop Closure,
  • the owned_reference it captures (a clone of the reference element), and
  • the update callback Rc (which, via the bindings, transitively pins the
    reference/floating node refs and signal setters).

In long-lived apps that mount/unmount many floating elements — e.g. a dropdown
that calls use_floating once per open cycle — leaked closures and detached
DOM nodes accumulate without bound. Cancelling the animation frame stops the CPU
cost but not the memory growth.

Reproduction

Mount a floating element with while_elements_mounted = auto_update (the default
path in the framework bindings), then repeatedly mount/unmount it (or toggle the
gating enabled signal). Take heap snapshots in DevTools across cycles: the
Closure instances and detached reference elements grow monotonically and are
never collected.

Suggested fix

Keep a separate handle to the cell and clear it in the returned cleanup, after
the animation frame is cancelled (so the closure can no longer run before it is
dropped):

let frame_loop_closure = Rc::new(RefCell::new(None));
let frame_loop_closure_clone = frame_loop_closure.clone();
let frame_loop_closure_cleanup = frame_loop_closure.clone();

// ... build the closure as before ...

Box::new(move || {
    // ... existing teardown ...

    if let Some(frame_id) = frame_id.take() {
        cancel_animation_frame(frame_id);
    }

    // Break the closure's self-referential Rc cycle so it (and the reference
    // element / update callback it captures) can be freed.
    drop(frame_loop_closure_cleanup.borrow_mut().take());
})

This is safe because the pending frame is cancelled first, and external cleanup
never runs concurrently with a RAF tick. I can open a PR with this fix if that
makes sense.

Possibly related

The same self-referential-Rc-closure pattern appears in observe_move's
refresh_closure in the same file, and its cleanup (the cleanup_rc) clears
observe_closure but not refresh_closure. That one looks trickier to fix
because the cleanup is also invoked from inside the refresh closure on each
refresh, so clearing the cell unconditionally would drop the running closure.
It likely needs the internal "refresh" teardown and the external teardown
separated. Flagging it here in case it's the same underlying bug — happy to file
it separately if you'd prefer.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions