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.

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

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

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

Comments

@alexhobbs
Copy link

@alexhobbs alexhobbs commented Aug 30, 2018

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
@babeal

This comment has been minimized.

Copy link

@babeal 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

This comment has been minimized.

Copy link

@AlexNarkevich AlexNarkevich commented Mar 1, 2019

image
we have the issue as well.

@alexhobbs

This comment has been minimized.

Copy link
Author

@alexhobbs alexhobbs commented Mar 25, 2019

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

@jpduckwo

This comment has been minimized.

Copy link

@jpduckwo jpduckwo commented Oct 21, 2019

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

@jpduckwo

This comment has been minimized.

Copy link

@jpduckwo jpduckwo commented Oct 22, 2019

@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.

crisbeto added a commit to crisbeto/angular that referenced this issue Dec 14, 2019
…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 added a commit to crisbeto/angular that referenced this issue Dec 14, 2019
…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 in 835ed0f Dec 16, 2019
kara added a commit that referenced this issue Dec 16, 2019
…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

This comment has been minimized.

Copy link

@denisyilmaz denisyilmaz commented Jan 12, 2020

this problem still persists in safari 13.0.4

@Splaktar

This comment has been minimized.

Copy link
Member

@Splaktar Splaktar commented Jan 15, 2020

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

@denisyilmaz

This comment has been minimized.

Copy link

@denisyilmaz denisyilmaz commented Jan 15, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can’t perform that action at this time.