Solvedangular Memory leak when FormControlName created/destroyed few times

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] 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

Current behavior

  1. FormControlName, objects provided by NG_VALIDATORS (may be some other) can not be collected by gc until there are refs for FormGroup.
    This is caused by collecting closures created in https://github.com/angular/angular/blob/4.4.6/packages/forms/src/directives/shared.ts#L33 in https://github.com/angular/angular/blob/4.4.6/packages/forms/src/model.ts#L744 .
  2. All provided validators (via NG_VALIDATORS token) are called as many times as form is recreated.

Expected behavior

  1. After destroying FormControlName it should be collected by gc even if there are refs for FormGroup.
  2. Provided validators should be removed from FormControl after destroying FormControlName.

Minimal reproduction of the problem with instructions

https://plnkr.co/edit/eCtf2naoxLe8bWTJ8IkV?p=preview (used example from material demo as starting point)

Steps to reproduce

  1. show/hide form clicking checkbox.
  2. displayed below number of listeners is increasing.
  3. there are more and more messages 'Validate called' appears in console when editing input (In my project I had issue when validators set outdated errors, had to use workaround).

There are 3 inputs to show that they are not synchronized (Is there a significant reason to store all closures, not only last?).

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

  1. Allow to have permanent FormControl/FormGroup without memory leaks.
  2. Allow to dynamically create FormControlName with custom control, which knows how to validate itself (providing NG_VALIDATORS), without calling validators of destroyed controls.

Environment

Angular version: 4.4.6


Browser:
- [x] Chrome (desktop) version 62.0.3202.75
- [ ] 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
 
Others:
25 Answers

✔️Accepted Answer

This issue has been around for nearly 3 years now.
(I usually don't like to start a message this way unless I tried something to fix the issue myself... Which I did here 😬 👼! and failed miserably as it seem quite complex to get to the bottom of it... But bear with me)

I do know that Ivy was necessary in order to solve a lot of issues in Angular ecosystem and that it ended up being really time and resource consumming for the Angular team to ensure that it was working well, that it was fast, that it was retro compatible, that ngcc could pre compile the libraries, etc. I trully appreciate the effort here 👏 👏 👏.

On the other hand, I think that like many other devs over the last few years, I've felt an important frustration with Angular.
Not because I wanted more brilliant features.
Not because Ivy wasn't there quite yet and I couldn't lazy load a component without using the router.
But rather because we've had annoying, recognized and long lived bugs, sometimes with lot of comments from the community to help debug or even PR made by the community to fix them (⚠️ !) pending for months or even years without any action or comment to fix them and bring them back into the main branch. So yes, pretty much since Ivy development started it's been quite a frustrating experience.

(I needed to get that off my chest at some point and did it into this issue, sorry 🤷‍♂️)

That said, I've always loved Angular community and always hoped that with Ivy going stable (I know the dev is not finished 100% on that though) it'd bring slowly but surely more focus on bug fixes, then new features 🙏. When I saw this tweet: https://twitter.com/angular/status/1265342327351939073 I was super happy as it felt like this was the beginning of it 🙌. Time will tell but still, I've seen a lot of efforts put into long lived and major issues (+ features!) so keep it up 💪 🔥 🙏.

Now, with the recent efforts from the Angular team to apparently tackle such important, annoying, long lived and forgotten issues I have hope that some light will be brought upon this one. This memory leak is making our app miserable 😞. Let me share why.

We have hundreds of forms into our app. We need to provide a lot of data from the "real world" to our app in order to run a simulation. Those forms are huge. In terms of data they contain and have to expose, types (input, radio buttons, checkboxes, select, ...). The UI components are also quite huge. For example we can have SVG rendered next to the form to represent some complex shapes. This way the user is able to visualise if the data he's entering make sense.

In order to manage all those data, we did try a lot of different solutions to break down our huge forms into smaller ones. We even have some "sub forms" that we can reuse in other forms 🔥. As this was critical for us and after spending an important amount of time to find a solution that was a nice fit for us, we decided to open source a library we built ourselves:
ngx-sub-form https://github.com/cloudnc/ngx-sub-form

This library internally uses ControlValueAccessor to build the sub forms and while I'm not 100% sure the issue is coming from there, I believe the odds are quite high. To give you an idea how bad it is for us with all the DOM elements that appear to never be removed, here's a recording of the performance monitor where I:

  • Open the app
  • Go to a page (let's call it P1) where we've got huge forms (containing lot of sub forms too)
  • Go to a different page (let's call it P2) where we also have huge forms (containing lot of sub forms too)
  • Go to P1
  • Go to P2
  • Go to P1
  • Go to P2

Just by doing that, have a look into the numbers for the DOM nodes and the event listeners 😱

ng-app-memory-leak-3

Just by opening 2 different pages, 3 times each, we end up with:

  • +80k detached DOM nodes
  • +4.9K detached JS event listeners

This never goes away and keeps increasing until we reload the page.

The overall feeling on the app is not pleasant for our users when after 6 clicks the memory footprint is like the following:

image

Now, with all the forms we've got, we may have forgotten something and I may be saying all that for nothing. But after trying to debug this for hours, making sure our observables are closed correctly when the components are destroyed and everything is well cleaned up etc, I can only see this happening on page containing forms (and sub forms using our library). Therefore I'm highly suspicious and think it may not be coming from us. If that's the case, it seems pretty bad and I hope the priority can be bumped so it gets looked at sooner than later.

Now, I know that just asking for something to be fixed is not enough especially when you can't provide a repro.
I can't give away the code of the app I mentioned above BUT we've built a demo app for ngx-sub-form in order to make sure that all the features are working correctly and we run some E2E tests onto that demo etc.

I checked on that demo which you can find here:

Note: the forms are much smaller on that demo, contain way less data and have less graphical elements. Therefore the memory leak doesn't seem as bad here but still.

Info: When clicking on an item on the left, it display into a form (composed of sub forms using ControlValueAccessor) the data

image

When we switch from one item to another, we get roughly +250 dettached DOM nodes and +100 JS event listeners. So still something that can be used to dig into the issue I assume.

I know everyone love a minimal repro too, so I also made one:

https://stackblitz.com/edit/ng-memory-leak-control-value-accessor?file=src/app/app.component.html

And here's the app live: https://ng-memory-leak-control-value-accessor.stackblitz.io/

If you open the chrome dev tool, press esc and a new panel will open. In the left menu on that panel, choose to display "Performance monitor" and click on the toggle button multiple times.

ng-app-memory-leak-4

Hope it helps 🙃

EDIT:

This may be related to:

Other Answers:

Hi,

Just want to give an update on memory leak issues with ReactiveForms directives.

Over the last few months we landed several updates to internal data structures in Forms package to have an ability to do proper cleanup when Forms-related directives are destroyed. With the upcoming fix in PR #39235 (that relies on the mentioned refactoring), the situation with memory leaks in ReactiveForms directives should be improved. We did verify that there are no memory leaks in common scenarios (and extended test coverage to check the cleanup logic), but if you get a chance, it'd be helpful if you could test the changes from PR #39235 in use-cases where you’ve observed the issue and let us know if memory leaks are gone and your app behaves as expected.

Here are few steps to test the changes from PR #39235 with your app:

  • make sure the app is upgraded to the latest major version (v11)
  • update the package.json file in the root folder of the app and use the following value for the ”@angular/forms” package:
  “@angular/forms: https://875212-24195339-gh.circle-artifacts.com/0/angular/forms-pr39235-5152d0fa00.tgz”,
  • Run ”npm install” to update the “@angular/forms” locally
  • Build your app and test the changes

Note: the mentioned URL is the “@angular/forms” package built by CI with the fixes from PR #39235. This package is NOT suitable for production usage and is available only for limited period of time (see more info on build artifacts in DEVELOPER.md file). The mentioned fix covers only ReactiveForms directives (such as [formGroup], [formConrol], formControlName, etc). If a memory leak that you see is related to the template-driven directives such as ngModel (and there is no regression with the fix from PR #39235), please check issue tracker for an existing issue or create a new ticket if such issue was not reported previously.

Thank you.

We've hit this bug.

We have our form's creation/destruction affected by an ngSwitch. If we use formControlName in our inputs, hiding and showing (creating and destroying) the form causes Subjects and SubjectSubscriptions to grow and never dispose.

It's been a hard time to debug this and memory leaks are always important.

I hope this gets fixed as soon as possible.

Thanks

I'm keen to give it a go @AndrewKushnir 👍
Thanks a lot for all the clean up and bug fixes you've been doing lately 🔥

I wanted to try first on the minimal repro I've built here https://stackblitz.com/edit/ng-memory-leak-control-value-accessor-ng11-fix-test but I got an error trying to update with “@angular/forms”: “https://875212-24195339-gh.circle-artifacts.com/0/angular/forms-pr39235-5152d0fa00.tgz”, on stackblitz.

So I just downloaded it and ran the project locally...

Before upgrading to “@angular/forms”: “https://875212-24195339-gh.circle-artifacts.com/0/angular/forms-pr39235-5152d0fa00.tgz”, After upgrading to “@angular/forms”: “https://875212-24195339-gh.circle-artifacts.com/0/angular/forms-pr39235-5152d0fa00.tgz”,
image image

And I'd say it's looking pretty good 🤩 🎉 🎉 🎉 🎉

Nice one @AndrewKushnir 👏!

Note: I don't think this would behave any differently on the app where I've got a crazy memory leak but I haven't been able to test on this one as we're still on angular 10

Honestly speaking,developing "fancy" new features may attract a few new users, but setting aside fundamental issues like this one are making more serious users abandoning Angular.

Related Issues:

608
angular Angular5.x lazyLoad problem, undefined is not a function
For others that find this issue via Google as i did: I had the same problem when trying to lazy load...
348
angular Cyclic dependency error with HttpInterceptor
I resolved simply not setting authService in constructor but getting in the intercept function. ...
277
angular Uncaught Error: Can't resolve all parameters for ...
You are missing an @Injectable() annotation on your ApiService Support requests like these should li...
266
angular Force reload/refresh current route with RouteReuseStrategy
Hi If you really need to trick the Router into reloading the component on each routerLink click ...
260
angular Misleading error message "Cannot find a differ supporting object '[object Object]'"
I just ran into the same issue I'm not sure if the recommended solution will work for my case ...
224
angular update 2 to 4 has problem [ts] Property 'map' does not exist on type 'Observable<Response>'.
I met the same problem with the angular cli 6.0.0 and rxjs 6.1.0 And I solved the problem by replaci...
170
angular Angular2 AoT Compiler Errors
pls try /cc @chuckjaz When I try to compile my project with ngc it throws the below error: Error: Er...
152
angular HttpClient.delete() cannot handle a body in its request
It would be great to have body param in .delete() We just migrated our project to HttpClient and for...
140
angular Http - Observable completed function not firing
Third callback haven't been called when error occures ES6 promises hasn't method finally only then a...
133
angular Using multiple components in different modules causing "Type X is part of the declarations of 2 modules" error
as @brandonroberts saids create a shared module like this: then use the SharedModule like this.. ...
112
angular Unsupported platform for fsevents@1.0.14: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})
@DzmitryShylovich did you edit package.json only? if npm-shrinkwrap.json is still there please remov...
98
angular Angular v5 ngc compiler: Error encountered in metadata generated for exported symbol 'Subscription'
Got this problem I'm submitting a.. Current behavior Building an Angulary library using 5.0.0-beta.4...
98
angular 404 on route refresh in angular 4
Got it working Just adding .htaccess in root I'm submitting a.. Current behavior I created a new com...
97
angular routerLinkActive not updating when routerLink changed
I have a hack that seems to work After looking at the source code it looks like this.update() is als...
97
angular No provider for HttpClient!
If you are using angular v5 version import HttpClientModule in your app.module.ts after HttpModule T...
81
angular ɵDomAnimationEngine and ɵNoopAnimationEngine module missing in animations 4.2.1
@dubedoy I installed @angular/animations@4.1.3 and it worked again. I'm submitting a .. ...
80
angular Error: Runtime compiler is not loaded in angular6 --prod mode
Do not and i repeat do not import your feature modules in app module and also when addressing featur...
79
angular appending header in HttpHeaders from '@angular/common/http' doesn't work
@trotyl I didn't understand your comment I'm submitting a.. ...
77
angular Can't bind to 'formGroup' since it isn't a known property of 'form'
did you import ReactiveFormsModule? I'm submitting a .. ...
73
angular AOT Compiler requires public properties, while non-AOT allows private properties
@aluanhaddad you have a big misunderstanding in here There is no subset of Typescript in here No one...
73
angular [Bug] angular/elements: Failed to construct 'HTMLElement': Please use the 'new' operator
Hi I have solved this issue by changing the target:es5 in the tsconfig.json to target:es2015 these i...
70
angular Cannot run angular 2+ from file:/// - looks like 'base href="/"' is the issue
Thanks @Markovy @audrummer15 I got it working completely in a fairly complex angular 2 app with mult...
69
angular HttpClient fails to parse an empty 200 response in IE11
For my error I was able to fix the problem by setting the responseType: to 'text' in the options ...
66
angular Function calls are not supported in decorators when fullTemplateTypeCheck is not specified and @dynamic has no effect
Regarding Ward's repro: @wardbell The build will succeed / fail depending on the combination of angu...
62
angular error TS2451: Cannot redeclare block-scoped variable 'ngDevMode'
had to add this line in the main tsconfig I'm submitting a.. ...
62
angular Problem with ngFor
Wouldn't [(ngModel)]=testItems[i] do the trick? I think that the error is saying that you can assign...
61
angular Issue with importing Observable from rxjs/Rx (import-blacklisted)
You shouldn't import from 'rxjs' or 'rxjs/Rx' since either import will import the whole of rxjs whic...
53
angular [RC5]: Minified bundle breaks
@robertoforlani Hopefully someone will have time to write a comprehensive explanation soon In the me...
53
angular router-outlet is appending rather than replacing when using BrowserAnimationsModule
Trying this solved the problem for me: this.zone.run(() => { this.router.navigate(['/main']); }); Re...
52
angular Lazy loaded module in named outlet throws error
We have this Where proxy route component is simply [x] bug report [ ] feature request [ ] support re...
52
angular IVY Error NG6002: Appears in the NgModule.imports of AppModule, but could not be resolved to an NgModule class
Not sure this will provide anyone relief or assist with figuring out what the root cause is but clea...
51
angular How to run angular 2 application on apache hosting server
Sorry to rock the boat I hope this doesn't attract more questions I'm only going to comment once :) ...
51
angular Support adding rel=canonical link tags using an included service
Eventually there will be some DocumentService part of Core that will handle both Meta/Link elements ...
50
angular Concept of Angular (ngZone + ChangeDetection) better than concept React, Vue (Virtual DOM)?
Concept of Angular (ngZone + ChangeDetection) better than concept React Vue (Virtual DOM)? If you ca...
50
angular I'd like to be able to use ngModel without specifying a name
Thank you all for the great feedback - very helpful! Here's how we are thinking about it: In the cas...
49
angular Model values not trimming automatically in angular 2
@laskoviymishka White space it already something If you are a programmer and think globally - yes ...
46
angular HttpClient - HttpErrorResponse not json but blob
I created this interceptor as a temporary solution until this one is fixed: I'm submitting a.. ...
45
angular Router's ActivatedRoute data returns empty {} if module is lazy
data is available only with this hell-like construction: And this is if you have children: ...
44
angular Misleading errormessage when using HostBinding with @animation trigger but no defined animations
The error message is not fine The error message says you're importing BrowserAnimationsModule incorr...
42
angular Memory leak when FormControlName created/destroyed few times
This issue has been around for nearly 3 years now (I usually don't like to start a message this way ...
41
angular Remove System.import() usage in favor of import()
I use a parser rule in my webpack configuration to disable the warning: https://webpack.js.org/confi...
41
angular Async event subscriber not updating UI after async call
Hi! The issue is that the async call result is outside ngZone thus not triggering the UI update You ...
40
angular Using router.navigate to navigate to another component does not invoke the onInit method
I have the same issue Angular is running in a Cordova app for iOS I tried the router-version 4.1.3 (...
37
angular Router: AoT compilation fails when using a function with loadChildren
Calling functions or calling new is not supported in metadata when using AoT This includes things li...
35
angular Provide a mock service using TestBed
I was having this issue as well however I noticed that my @component metadata still had the provider...
35
angular Angular2 download excel file from Web API, file is corrupt
@healkar01 I had the same issue and I resolved using native angular2 http request in this way: Backe...
35
angular 4.0.0-rc.6 [platform-server] - Cannot find module '@angular/animations/browser'. & other errors
(Just incase others find it) Make sure @angular/animations is installed as a dependency and the erro...
35
angular HttpClient mapping to typescript types not working
I agree with all the previous comments I find the syntax misleading widget.service.zip widget.servic...