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

Memory Leak on Modals after dismiss #16648

Closed
doug1e opened this issue Dec 9, 2018 · 18 comments
Closed

Memory Leak on Modals after dismiss #16648

doug1e opened this issue Dec 9, 2018 · 18 comments
Labels
package: core @ionic/core package

Comments

@doug1e
Copy link

doug1e commented Dec 9, 2018

Bug Report

Ionic version:

[x] 4.0.0-beta.13

Current behavior:

When Modal (pop up) is displayed and then dismissed, it leaved Detached HTMLElement (memory leaks) behind.

Expected behavior:

Modal dismiss should clean up the memory allocation

Steps to reproduce:

The sample given at the docs is enough to produce the problem. When you have larger components used as the base for the modal, the issue becomes more dramatic.

Related code:

The code to launch the modal:

export class HomePage {
constructor(public modalController: ModalController) {
}

async presentProfileModal() {        
    const modal = await this.modalController.create({
        component: TestModalComponent,
        componentProps: { value: 123 }
    });

    await modal.present();
}         

}

Please find the component code below. The only added method is related to modal dismiss.

closeModal() {
this.modalController.dismiss();
}


Other information:

Ionic info:

Ionic:

   ionic (Ionic CLI) : 4.2.1

System:

   NodeJS : v8.9.4
   npm    : 6.4.1
   OS     : Windows 7
@ionitron-bot ionitron-bot bot added the triage label Dec 9, 2018
@manucorporat
Copy link
Contributor

Can you please follow the issue template? Ionic 4.2.1 is not the ionic version, but the CLI version

@manucorporat manucorporat added the needs: reply the issue needs a response from the user label Dec 9, 2018
@ionitron-bot ionitron-bot bot removed the triage label Dec 9, 2018
@doug1e
Copy link
Author

doug1e commented Dec 9, 2018

Apologies about the template; I edited the report.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Dec 9, 2018
@manucorporat
Copy link
Contributor

@Doehl we might already fixed this issue in master, next version should have it

@paulstelzer
Copy link
Contributor

@doug1e I leave this open for needs reply so you can check with the next beta which is coming soon if the problem is solved and then it would be great if you can close this issue if it's fixed :)

@paulstelzer paulstelzer added needs: reply the issue needs a response from the user and removed triage labels Dec 11, 2018
@manucorporat
Copy link
Contributor

Good idea @paulstelzer

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Dec 11, 2018
@manucorporat manucorporat added the needs: reply the issue needs a response from the user label Dec 11, 2018
@ionitron-bot ionitron-bot bot removed the triage label Dec 11, 2018
@doug1e
Copy link
Author

doug1e commented Dec 11, 2018

I will, many thanks for all your help. I do appreciate it

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Dec 11, 2018
@paulstelzer paulstelzer added needs: reply the issue needs a response from the user and removed triage labels Dec 11, 2018
@godenji
Copy link

godenji commented Dec 13, 2018

check with the next beta which is coming soon

When is the next beta coming? Quite a few commits against master, hoping to test out latest fixes/improvements this week.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Dec 13, 2018
@paulstelzer
Copy link
Contributor

paulstelzer commented Dec 13, 2018

If you can't wait, just fork and build the repo :) There are many new features and fixes. We hope to release beta.18 soon :)

@paulstelzer paulstelzer added needs: reply the issue needs a response from the user and removed triage labels Dec 13, 2018
@brandyscarney
Copy link
Member

4.0.0-beta.18 has been released, could you please test this and let us know if it's still an issue? Thanks!

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Dec 13, 2018
@brandyscarney brandyscarney added the needs: reply the issue needs a response from the user label Dec 13, 2018
@ionitron-bot ionitron-bot bot removed the triage label Dec 13, 2018
@doug1e
Copy link
Author

doug1e commented Dec 15, 2018

Many thanks for your answers.

I might be doing something wrong but I updated to 4.0.0-beta.19 and I still see the same behavior.

image

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Dec 15, 2018
@paulstelzer
Copy link
Contributor

I cannot see something wrong in your screenshot. Where do you see the memory leak??

@paulstelzer paulstelzer added needs: reply the issue needs a response from the user and removed triage labels Dec 20, 2018
@doug1e
Copy link
Author

doug1e commented Dec 20, 2018

Hi Paul,

Many thanks for your reply and congrats on the RC release.

As far as I know the Detached HTMLElement is a memory leak, which can be seen at the screenshot.
I can also replicate a similar behavior with ToastController sample code.

I believe, these leaks may easily add up during the life of the app.

Cheers,

Doug

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Dec 20, 2018
@ionitron-bot ionitron-bot bot removed the triage label Dec 23, 2018
@doug1e
Copy link
Author

doug1e commented Dec 31, 2018

Hi ionic team,

I wish you and your loved ones a wonderful 2019.

I was wondering if this bug is planned to be fixed before the production version. My app uses toast messsages and popups quite a bit; hence, the memory leaks issue is a bit of a concern for me...

Cheers,

Doug

@manucorporat
Copy link
Contributor

Happy new year @doug1e!
are you using stencil or ionic/angular?

@doug1e
Copy link
Author

doug1e commented Jan 1, 2019

Hi Manu,

I use ionic/angular

Cheers,

Doug

@doug1e
Copy link
Author

doug1e commented Jan 22, 2019

Hi ionic team,

Is there anything I can do to help with the debugging of this issue?

Cheers,

Doug

@doug1e
Copy link
Author

doug1e commented Jan 24, 2019

Hi @manucorporat ,

Congrats on the ionic 4 release and thank you so much for the hard work.

I have been working on a project that uses modals and toasts so I was wondering how you prioritize this bug. Do you think it will be looked at soon? The reason why I am asking this is to figure out whether to publish my app now... Although memory leaks causes issues, I am not sure how long I need to wait so wanted to check with you.
Any pointer would be greatly appreciated.

Thanks again for your time on this and for the ionic 4 release.

Cheers,

Doug

@ionitron-bot
Copy link

ionitron-bot bot commented Jan 24, 2020

Thanks for the issue! This issue is being closed due to inactivity. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

Thank you for using Ionic!

@ionitron-bot ionitron-bot bot closed this as completed Jan 24, 2020
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Jan 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package
Projects
None yet
Development

No branches or pull requests

5 participants