Skip to content

fix(animations): remove finish listener once player is destroyed#51136

Closed
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/animations-leak
Closed

fix(animations): remove finish listener once player is destroyed#51136
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/animations-leak

Conversation

@arturovt
Copy link
Contributor

@arturovt arturovt commented Jul 21, 2023

This commit removes the finish listener from the Animation object once the animation is finished, effectively resolving a memory leak. Previously, the finish listener captured this, which prevented this from being garbage collected.

See the image below:

Screenshot from 2023-07-22 00-13-37

@pullapprove pullapprove bot requested a review from crisbeto July 21, 2023 21:24
@arturovt arturovt force-pushed the fix/animations-leak branch from 4c2cc25 to 1171fea Compare July 21, 2023 21:32
Copy link
Member

@JeanMeche JeanMeche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Depending on which one will get merged first between this one and #50662. We'll need a rebase;

@ngbot ngbot bot added this to the Backlog milestone Aug 9, 2023
This commit removes the `finish` listener from the Animation object once
the animation is finished, effectively resolving a memory leak. Previously,
the `finish` listener captured `this`, which prevented `this` from being garbage collected.
@arturovt arturovt force-pushed the fix/animations-leak branch from 1171fea to 9b8cd13 Compare September 8, 2023 18:36
@JoostK JoostK added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Nov 2, 2023
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 2, 2023
@alxhub
Copy link
Member

alxhub commented Nov 2, 2023

This PR was merged into the repository by commit a02a745.

alxhub pushed a commit that referenced this pull request Nov 2, 2023
…51136)

This commit removes the `finish` listener from the Animation object once
the animation is finished, effectively resolving a memory leak. Previously,
the `finish` listener captured `this`, which prevented `this` from being garbage collected.

PR Close #51136
@alxhub alxhub closed this in a02a745 Nov 2, 2023
alxhub pushed a commit that referenced this pull request Nov 2, 2023
…51136)

This commit removes the `finish` listener from the Animation object once
the animation is finished, effectively resolving a memory leak. Previously,
the `finish` listener captured `this`, which prevented `this` from being garbage collected.

PR Close #51136
@arturovt arturovt deleted the fix/animations-leak branch November 2, 2023 17:39
Abseil-byte pushed a commit to Abseil-byte/angular that referenced this pull request Nov 5, 2023
…ngular#51136)

This commit removes the `finish` listener from the Animation object once
the animation is finished, effectively resolving a memory leak. Previously,
the `finish` listener captured `this`, which prevented `this` from being garbage collected.

PR Close angular#51136
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 3, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…ngular#51136)

This commit removes the `finish` listener from the Animation object once
the animation is finished, effectively resolving a memory leak. Previously,
the `finish` listener captured `this`, which prevented `this` from being garbage collected.

PR Close angular#51136
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: animations target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Comments