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:

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 😱

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:

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:

• Live demo https://cloudnc.github.io/ngx-sub-form
• Code https://github.com/cloudnc/ngx-sub-form/tree/master/src/app

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

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.

Hope it helps 🙃

EDIT:

This may be related to:

• #37431
• #37598
• #37671
• #37952

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”,

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.