Ionic-framework: DOM Node Leak for modal & nav push / pop

[mikethm]

Ionic version: (check one with "x")
[ ] 1.x (For Ionic 1.x issues, please use https://github.com/ionic-team/ionic-v1)
[ ] 2.x
[X] 3.x

I'm submitting a ... (check one with "x")
[X] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or http://ionicworldwide.herokuapp.com/

Current behavior:
Nodes always seem to increase with modal open and dismiss even after garbage collection..

Expected behavior:
nodes count would fall when modal dismissed.

Steps to reproduce:
run ionic conference app
https://github.com/ionic-team/ionic-conference-app

skip the intro. repeatedly open and close the filter settings modal.
(https://monosnap.com/file/xg1mpk9aaHaCoMbSIUIt53WifB55p8) button at the top right.
Monitor using performance tab on chrome developer tools -- shows the nodes increasing every time modal is opened

https://monosnap.com/file/xIFbiboFMyAXdIyBNCBUWVNpGStI11

Related code:

Other information:
in the middle, i pressed the button for garbage collection - nodes only went down a tiny bit.. seems to add about 250 nodes every time the modal opens.
i pressed model open and closed 10 times, then garbage collection, then open and close another 10 times. at the end there are close to 6000 nodes....

In other apps, also seems to happen when i use navctrl push and pop on other apps. nodes do not appear to be released, even in --prod mode.

Ionic info: (run ionic info from a terminal/cmd prompt and paste output below):

global packages:

    Cordova CLI : 7.0.1 

local packages:

    @ionic/app-scripts : 2.1.3
    Cordova Platforms  : none
    Ionic Framework    : ionic-angular 3.6.0

System:

    Node       : v7.10.0
    OS         : macOS Sierra
    Xcode      : Xcode 8.3.3 Build version 8E3004b 
    ios-deploy : 1.9.1 
    ios-sim    : 6.0.0 
    npm        : 5.3.0 

[jgw96]

Hello, thanks for using Ionic! Unfortunately, I am not seeing the same issue when i trace this.

As you can see in my timeline, nodes and JS heap go up for a while, and then chrome garbage collects and things go back down. This is in the latest chrome with the conference app.

[mikethm]

@jgw96 Launching the modal window 3 times is not enough to see the problem. as the garbage collection seems to collect a few nodes, depending.

however, if you do 10 times, garbage collect, then another 10 times garbage collect, then look at the trace

there is a huge buildup over time. I have tried this across multiple projects and the behaviour is the same, and i'm confused why it could be.

Am I able to change the tags? it is relevant to v3, i think...

[dylanvdmerwe]

Any further info on this?

[mikethm]

Are you experiencing it too? maybe let the team know.

I used to have "on iOS compiled apps, for example, will cause the device to freeze after a few cycles of this, depending on the machine." on the original description, but that was due to another bug

[dylanvdmerwe]

Can you elaborate on the other bug?

[mikethm]

12737

[kensodemann]

Hello! Thank you for opening an issue with us! Would you be able to provide a sample application via GitHub that demonstrates the issue you are having?

That would greatly help however picks up this issue.

Thanks for using Ionic!

[jbgomez21]

Hi @kensodemann

https://github.com/jbgomez21/modal-memory-leak

Regards

[longgt]

src/components/modal/modal-component.ts

ionViewPreLoad() {
    // ******** DOM WRITE ****************
    const componentRef = this._viewport.createComponent(componentFactory, this._viewport.length, this._viewport.parentInjector, []);
}

The componentRef in the code above should be saved somewhere and destroyed with modal component.
I have tried to implement something like this and component used in modal disappeared from heap snapshot.

Other area in Ionic source has same logic.

• srccomponentspopoverpopover-component.ts
• srccomponentsappapp-root.ts

Beside that, ModalImp, ModalCmp object is not destroyed, still existed in heap snapshot because leak in destroy, remove event listener.

Some logic can be considered as leak

srcnavigationview-controller.ts

EventListener

constructor() {
window.addEventListener('orientationchange', this.handleOrientationChange.bind(this));
}

Always add but not remove. (After navigated to n page, checked by window.__zone_symbol__orientationchangefalse in Chrome Dev Tools, the length of array was not decreased.

ElementRef

_ionCntDir, _ionCntRef

Assigned with _cntDir, _cntRef but was not set to null on destroy.

EventEmitter

Used to subscribe but did not call unsubcribe.

srccomponentsion.ts
ElementRef

Assigned in constructor but was not set to null on destroy.

By implementing all fix of above problems, ModalImp, ModalCmp is removed completely from heap snapshot.

[dylanvdmerwe]

Very nice find. Hopefully this can get sorted out quite quickly.

It would be great for other areas in the framework to be looked at for similar memory leaks.

[longgt]

It seems to be related with this

https://github.com/angular/angular/issues/22155

[walljm]

it also seems related to this: Browser Memory isn't released when destroying components #13725

I'm experiencing the same problem. I have a basic list of cards. if you add then remove the same card each time the dom nodes goes up and stays up. i'm basically adding and removing an item to an array, and angular is making the changes. In the below graphic i did this 3-4 times. i've done it with more cards and for longer, the dom nodes never go down.

[jrmlt]

I don't know about known angular memory leak issues, but I just stumble upon this particular one due to handleOrientationChange handler not released.
The problem is the use of .bind(this) when binding to 'orientationchange' event. When using .bind(this) a new reference is returned each time which differs from the previous .bind(this) (see this little fiddle I have created https://jsfiddle.net/2tzj9o0L/8/).
So when calling removeEventListener in the destroy callback, the handler is not the same as the one passed to the addEventListener.

Using the registerListener utils of the Platform class or keeping the reference this._bindHandler = this.handleOrientationChange.bind(this) for example solved the issue and DOM Nodes are collected.

I didn't check if there are others .bind(this) issue like this one. Might be a good idea to make a little review.

[meisterlampe]

Having the same issue with a long running Ionic-Application here.
Any chance to get a Bugfix version of Ionic 3 with this fix?
What would be the easiest way to get this changes into my project using Ionic 3.9.2?

Every hint is welcome :)

[michael-may]

@meisterlampe

I just ran into this on ionic-angular 3.9.2. I just put a postinstall script in my package.json that replaces /node_modules/ionic-angular/navigation/view-controller.js with @jrmlt's patched file above.

Not the most elegant solution, but it'll work until (if) 3.9.x is patched.

[meisterlampe]

Thanks for sharing. I guess I'll go this way too :)

[speence]

Any update on this?

[speence]

Just realized as per comments on @jrmlt patch that no more new features will be merged in to v3. Does anyone know if this has been patched in v4? Before I attempt to migrate my app, wondering if anyone has tried this :)

[dylanvdmerwe]

Maybe try create a repo in a v4 app to see if this still happens after measuring. I doubt it though as v4 is pretty much just a normal angular app.

[ionitron-bot[bot]]

This issue has been automatically identified as an Ionic 3 issue. We recently moved Ionic 3 to its own repository. I am moving this issue to the repository for Ionic 3. Please track this issue over there.

If I've made a mistake, and if this issue is still relevant to Ionic 4, please let the Ionic Framework team know!

Thank you for using Ionic!

[ionitron-bot[bot]]

Issue moved to: https://github.com/ionic-team/ionic-v3/issues/231