Skip to content

fix(dom): break frame-loop closure Rc cycle in auto_update cleanup#357

Open
ademilsonfp wants to merge 1 commit into
RustForWeb:mainfrom
ademilsonfp:bugfix/auto-update-frame-loop-closure-leak
Open

fix(dom): break frame-loop closure Rc cycle in auto_update cleanup#357
ademilsonfp wants to merge 1 commit into
RustForWeb:mainfrom
ademilsonfp:bugfix/auto-update-frame-loop-closure-leak

Conversation

@ademilsonfp

Copy link
Copy Markdown

Summary

Fixes #349.

auto_update builds a self-rescheduling animation-frame closure that is stored
in an Rc<RefCell<Option<Closure>>> and captures a clone of that same Rc so
it can reschedule itself, forming a strong reference cycle. The cleanup returned
by auto_update only cancelled the pending animation frame and never cleared
the cell, so the cycle was never broken — every call leaked the closure together
with the reference element clone and the update callback it captures. The
closure is built unconditionally, so the leak happened in both animation_frame
modes.

Fix

Keep a separate handle to the cell and drop its contents in the returned
cleanup, after the animation frame is cancelled (so the closure can no
longer run before it is dropped). This breaks the cycle and lets the closure and
everything it captures be freed.

Safety

The pending frame is cancelled before the closure is dropped, so there is no
use-after-free in wasm, and external cleanup never runs concurrently with a RAF
tick.

Verification

  • cargo check -p floating-ui-dom --target wasm32-unknown-unknown compiles.
  • cargo clippy -p floating-ui-dom reports no new warnings.

Note

A sibling instance of the same self-referential-Rc-closure pattern exists in
observe_move's refresh_closure (see the "Possibly related" section of #349).
It is trickier because its cleanup is also invoked from inside the refresh
closure, so it is intentionally left out of this PR and can be addressed
separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant