fix(helpers): guard addChild against re-inserting an already-linked node#1163
Open
terminalchai wants to merge 1 commit into
Open
fix(helpers): guard addChild against re-inserting an already-linked node#1163terminalchai wants to merge 1 commit into
terminalchai wants to merge 1 commit into
Conversation
Re-adding a tween that is already present in the composition replace list causes addChild to produce a self-referential linked list cycle (child._nextRep === child). The sorted-walk inside addChild can land on the child itself as prev, then sets prev[nextProp] = child which makes the child point to itself, freezing the animation engine update loop. Fix: at the top of addChild, detect whether the child is already linked in this list (parent._head === child, meaning it is the head, or child[prevProp] != null, meaning it has a predecessor). When detected, call removeChild first so the node is cleanly unlinked before being re-inserted at its new sorted position. The default value of _prevRep / _nextRep on a fresh tween is undefined, so the != null guard (loose inequality) correctly ignores unlinked nodes while still catching both null (after a previous removeChild) and an object reference (still linked). Fixes juliangarnier#1138
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Closes #1138
When overlapping
replacetweens are created on the same target/property in rapid succession (e.g. promise callbacks racing withdocument.hiddenforeground events),addChildcan be called with a tween that is already present in the composition replace list. This produces a self-referential linked list cycle (tween._prevRep === tween) that freezes the animation engine update loop, causing a tab hang.The issue was observed in devtools as the same tween ID repeating thousands of times in the
_prevRepchain.Root cause
addChildinserts unconditionally. Ifchildis already in the list its_prevRep/_nextReppointers are non-null, so the sorted-walkwhile (prev && sortMethod && ...)will eventually reach the child itself asprev. At that point:Fix
Add an early guard at the top of
addChildthat detects whether the child is already linked and callsremoveChildfirst:parent._head === child— child is the head of the list (its_prevRepis null but it is already inserted)child[prevProp] != null— child has a predecessor, meaning it is mid-list or tailremoveChildcleanly unlinks the node (setting both pointers tonull) beforeaddChildre-inserts it at the correct sorted position. When called with the default_prev/_nextprops this also protects the main engine ticking list against the same edge case.Files changed
src/core/helpers.js— source fixdist/— rebuilt