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

Dialog causes a memory leak when Dialog component contains mat-form-field #15823

Closed
stdcall opened this issue Apr 15, 2019 · 11 comments
Closed
Assignees
Labels
animation This issue is related to Angular animations or CSS animations P2 The issue is important to a large percentage of users, with a workaround perf This issue is related to performance

Comments

@stdcall
Copy link

stdcall commented Apr 15, 2019

What is the expected behavior?

Instantiating a dialog with a component that contains mat-form-field element causes a DOM node memory leak.

What is the current behavior?

Memory leak

What are the steps to reproduce?

https://stackblitz.com/edit/angular-yx1kuw?file=app%2Fdialog-overview-example-dialog.html

Try to open and close the dialog several times. Notice each time the number of detached DOM nodes gets higher and forcing garbage collection doesn't reduce it.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Angular Material: 7.3.7
Chrome: 73.0.3683.103

Is there anything else we should know?

Could be related to #10188

@stdcall stdcall changed the title Dialog causes a memory leak when Dialog components contains input Dialog causes a memory leak when Dialog component contains input Apr 15, 2019
@stdcall stdcall changed the title Dialog causes a memory leak when Dialog component contains input Dialog causes a memory leak when Dialog component contains mat-form-field Apr 17, 2019
@andrewseguin andrewseguin added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label May 9, 2019
@rhartvig
Copy link

rhartvig commented Jun 4, 2019

This is caused by @angular/animations package failing to clean up cached elements. The issue occurs when both a parent component (in this case dialog-container) and a child component (mat-form-field-hint-wrapper) has a set of animation triggers. A couple of issues tracking leaks are potentially related:

angular/angular#30728
angular/angular#30693

Contrary to other error cases, every time Dialog component is opened, a new set of elements is cached and subsequently leaked. This is due to a new namespaceId being created each time a dialog is opened. Thus the leaks build up rather than taper off in this case.

The layout is roughly like so:

<dialog container> <-- has triggers
   <mat-form-field>
      <mat-form-field-hint-wrapper>  <-- has triggers

An issue should probably be raised for @angular/animations package

@njofce
Copy link

njofce commented Sep 29, 2019

@rhartvig The memory leak still persists even after removing the animations module

@rhartvig
Copy link

@njofce , I guess you're using NoopAnimationsModule instead of BrowserAnimationsModule? If so, I guess that doesn't change the offending code paths. IIRC the issue is in
packages/animations/browser/src/render/transition_animation_engine.ts (https://github.com/angular/angular/blob/master/packages/animations/browser/src/render/transition_animation_engine.ts) where the cache structures would not get cleared for all the keys used.

@njofce
Copy link

njofce commented Sep 30, 2019

@rhartvig Yes, you are right.

@jpduckwo
Copy link

Hi, does anyone have a workaround for this? We have a lot of forms inside dialogs in our app and seeing a huge build up of unreleased nodes.

@njofce
Copy link

njofce commented Oct 21, 2019

@jpduckwo Try to use ngModel instead of FormGroups/FormControl

@rhartvig
Copy link

rhartvig commented Oct 21, 2019

@jpduckwo , I can offer a workaround that address the leaks I am seeing:

https://stackblitz.com/edit/angular-yx1kuw-fgdzjg

Notice in dialog component constructor:

 this.isClosing$ = this.dialogRef.beforeClosed().pipe(mapTo(true));

and surrounding dialog content:

<ng-container *ngIf="!(isClosing$ | async)">
...

Ie. a manual destroy of dialog content before dialog is closed.

To see how this makes a difference:

  1. From devtools press ctrl+p (or Cmd+p) to open the file transition_animation_engine.ts
  2. Set a breakpoint in clearElementCache around the line:
const elementPlayers = this._engine.playersByElement.get(element);
  1. Open and close dialog multiple times and notice how this._engine.playersByElement does not continue to build up elements, contrary to original stackblitz.

@jpduckwo
Copy link

Thanks so much guys, that's a great idea I'll give it a test in our app. I'm thinking of having a stab at creating a pull request to fix this issue, bit crazy when you think about it really...

@jpduckwo
Copy link

I believe this could be the root cause issue: angular/angular/issues/25744

@Splaktar Splaktar added P2 The issue is important to a large percentage of users, with a workaround perf This issue is related to performance animation This issue is related to Angular animations or CSS animations and removed P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Dec 13, 2019
@crisbeto
Copy link
Member

Should've been fixed in 9.0.0-rc.7 by angular/angular@835ed0f.

@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 Jan 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
animation This issue is related to Angular animations or CSS animations P2 The issue is important to a large percentage of users, with a workaround perf This issue is related to performance
Projects
None yet
Development

No branches or pull requests

7 participants