Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Animations: elements with leave transitions never cleanup causing memory leaks #25744

Closed
alexhobbs opened this issue Aug 30, 2018 · 9 comments
Closed

Animations: elements with leave transitions never cleanup causing memory leaks #25744

alexhobbs opened this issue Aug 30, 2018 · 9 comments
Labels
area: animations freq3: high hotlist: google regression Indicates than the issue relates to something that worked in a previous version state: has PR type: bug/fix
Milestone
Backlog

Comments

@alexhobbs
Copy link

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

When a component has a :leave transition the DOM Element reference is never removed from TransitionAnimationEngine.statesByElement map causing detached DOM nodes and out-of-memory crashes.

Debugging seems to show that the "hasAnimation" flag is preventing cleanup functions being run (see _flushAnimations). Elements never re-enter this function and therefore removeNodesAfterAnimationDone or processLeaveNode are not triggered.

Expected behavior

TransitionAnimationEngine should successfully clean up elements that have left the page.

Minimal reproduction of the problem with instructions

  1. Create ListItemComponent which has a simple :leave transition in the @component decorator
  2. Include this component with an *ngIf toggle attached to a button in the main AppComponent to trigger the :leave animation
  3. Watch detached DOM nodes grow and TransitionAnimationEngine.statesByElement grow

What is the motivation / use case for changing the behavior?

Currently this memory leak is causing performance issues and crashes on dashboard monitor type applications which run for a long time with no user input. Over the cause of a day statesByElement has grown from 0 elements to 10,000.

Environment


Angular version: 6.1.4

 
Browser:
- [x] Chrome (desktop) version 68
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
@ngbot ngbot bot added this to the needsTriage milestone Sep 4, 2018
@matsko matsko added type: bug/fix freq3: high regression Indicates than the issue relates to something that worked in a previous version labels Sep 27, 2018
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Sep 27, 2018
@babeal
Copy link

babeal commented Feb 28, 2019

@alexhobbs we are running into the same issue. What did you do to fix it in the mean time? Pure css animations?

@AlexNarkevich
Copy link

image
we have the issue as well.

@alexhobbs
Copy link
Author

@babeal - yes where possible we’ve had to replace with CSS or disable the animation entirely.

This was referenced Jul 7, 2019
Closed
Closed
@jpduckwo
Copy link

Hi, we are having issues with angular material because of this bug. Specifically MatFormField inside MatDialog.

@jpduckwo jpduckwo mentioned this issue Oct 22, 2019
Closed
@jpduckwo
Copy link

@matsko have you got any ideas on how to approach fixing this one? I'm willing to do the work if there is a direction to take. At worst opening up some way to call a clean up on TransitionAnimationEngine.statesByElement.

@rudzikdawid rudzikdawid mentioned this issue Dec 8, 2019
Closed
@Splaktar Splaktar mentioned this issue Dec 13, 2019
Closed
crisbeto added a commit to crisbeto/angular that referenced this issue Dec 14, 2019
@crisbeto
fix(animations): leaking detached nodes when parent has a leave trans…
7083000 
…ition

In the `TransitionAnimationEngine` we keep track of the existing elements with animations and we clear the cached data when they're removed. We also have some logic where we transition away the child elements when a parent is removed, however in that case we never cleared the cached element data which resulted in a memory leak. The leak is particularly visible in Material where whenever there's an animated overlay with a component inside of it that has an animation, the child component would always be retained in memory.

Fixes angular#25744.
@crisbeto crisbeto mentioned this issue Dec 14, 2019
Closed
crisbeto added a commit to crisbeto/angular that referenced this issue Dec 14, 2019
@crisbeto
fix(animations): leaking detached nodes when parent has a leave trans…
3c9c360 
…ition

In the TransitionAnimationEngine we keep track of the existing elements with animations and we clear the cached data when they're removed. We also have some logic where we transition away the child elements when a parent is removed, however in that case we never cleared the cached element data which resulted in a memory leak. The leak is particularly visible in Material where whenever there's an animated overlay with a component inside of it that has an animation, the child component would always be retained in memory.

Fixes angular#25744.
@kara kara closed this as completed in 835ed0f Dec 16, 2019
kara pushed a commit that referenced this issue Dec 16, 2019
@crisbeto @kara
fix(animations): leaking detached nodes when parent has a leave trans…
6607fb4 
…ition (#34409)

In the TransitionAnimationEngine we keep track of the existing elements with animations and we clear the cached data when they're removed. We also have some logic where we transition away the child elements when a parent is removed, however in that case we never cleared the cached element data which resulted in a memory leak. The leak is particularly visible in Material where whenever there's an animated overlay with a component inside of it that has an animation, the child component would always be retained in memory.

Fixes #25744.

PR Close #34409
@denisyilmaz
Copy link

this problem still persists in safari 13.0.4

@Splaktar
Copy link
Member

@denisyilmaz using 9.0.0-rc.7 or newer? If you have a reproduction, please open an issue and link to this one.

@denisyilmaz
Copy link

9.0.0-rc.7 yes. I will try to create a reproduction asap.

@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 Feb 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area: animations freq3: high hotlist: google regression Indicates than the issue relates to something that worked in a previous version state: has PR type: bug/fix
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

fix(animations): leaking detached nodes when parent has a leave transition crisbeto/angular
10 participants
@matsko @IgorMinar @jelbourn @alexhobbs @Splaktar @denisyilmaz @crisbeto @babeal @jpduckwo @AlexNarkevich