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

bug: conditional rendering with ionic component causes memory leak #5172

Open
18 of 78 tasks
Lucas-Quinn-1163273 opened this issue Dec 14, 2023 · 6 comments
Open
18 of 78 tasks
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@Lucas-Quinn-1163273
Copy link

Lucas-Quinn-1163273 commented Dec 14, 2023

Prerequisites

Stencil Version

@stencil/core at all versions, 2.x, 3.x, 4.x and the latest 4.8.2. Haven't tried 1.x.

Current Behavior

I have identified two types of memory leaks so far. The first type occurs in the development environment and is caused by components like ion-button.

The second type is more complex, but more importantly, it happens in prod build too - this has been moved to #5181

Let's focus on the memory leak in the development environment in this issue.

The First Kind of Leaking

When rendering a component conditionally, certain components, such as ion-button, trigger a memory leak. If the component exists in a Stencil component and is rendered conditionally, the entire Stencil component fails to be garbage collected. For instance, wrapping ion-button in a Stencil component and rendering it conditionally prevents the garbage collection of the Stencil component.

While this type of leaking is limited to the development environment, distinguishing between development-only and production-related leaks is challenging. This ambiguity significantly impacts the development experience, requiring extensive time spent on debugging memory leaks. To determine whether a component is leaking, one must build and run the app in production mode, adding a considerable overhead to the debugging process.

Environment details:

  • Ionic@latest

  • React@latest: Perfectly clean

  • Angular@latest: Perfectly clean

  • Vue@latest: One element left in memory, but it cleans memory correctly

  • Stencil@latest: Unable to perform garbage collection correctly, resulting in memory leaks

To reproduce the issue:

  1. Run npm install.
  2. Execute npm run start.
  3. Click the button to observe the memory leak issue.
Screenshot 2023-12-15 at 11 16 56 AM Screenshot 2023-12-15 at 11 18 22 AM

Replace ion-button with other components to observe the same issue:

Expand to see components
  • ion-action-sheet
  • ion-accordion
  • ion-accordion-group
  • ion-alert
  • ion-badge
  • ion-breadcrumb
  • ion-breadcrumbs
  • ion-button
  • ion-ripple-effect
  • ion-card
  • ion-card-content
  • ion-card-header
  • ion-card-subtitle
  • ion-card-title
  • ion-checkbox
  • ion-chip
  • ion-app
  • ion-content
  • ion-datetime
  • ion-datetime-button
  • ion-picker
  • ion-fab
  • ion-fab-button
  • ion-fab-list
  • ion-grid
  • ion-col
  • ion-row
  • ion-infinite-scroll
  • ion-infinite-scroll-content
  • ion-icon
  • ion-input
  • ion-textarea
  • ion-item
  • ion-item-divider
  • ion-item-group
  • ion-item-sliding
  • ion-item-options
  • ion-item-option
  • ion-label
  • ion-note
  • ion-list
  • ion-list-header
  • ion-avatar
  • ion-icon
  • ion-img
  • ion-thumbnail
  • ion-menu
  • ion-menu-button
  • ion-menu-toggle
  • ion-split-pane
  • ion-modal
  • ion-backdrop
  • ion-nav
  • ion-nav-link
  • ion-router-outlet
  • ion-route
  • ion-route-redirect
  • ion-searchbar
  • ion-segment
  • ion-segment-button
  • ion-select
  • ? ion-select-option with ion-select
  • ion-tabs
  • ? ion-tab with ion-nav inside
  • ion-tab-bar
  • ion-tab-button
  • ion-toast
  • ion-toggle
  • ion-toolbar
  • ion-header
  • ion-footer
  • ion-title
  • ?ion-buttons (must have ion-button, so it has an issue)
  • ion-back-button
  • ion-text

The Second Kind of Leaking

Moved to #5181

Expected Behavior

Detached elements should be garbage collected.

System Info

npx @stencil/core info

System: node 21.4.0
    Platform: darwin (23.2.0)
   CPU Model: Apple M2 (8 cpus)
    Compiler: /Users/[username]/.npm/_npx/0eea9657e3e2e6b9/node_modules/@stencil/core/compiler/stencil.js
       Build: 1702322155
     Stencil: 4.8.2 🐳
  TypeScript: 5.2.2
      Rollup: 2.42.3
      Parse5: 7.1.2
      Sizzle: 2.42.3
      Terser: 5.26.0

Tried different browsers, different systems, different node versions, different typescript versions too.

Steps to Reproduce

Clone the repo

To reproduce the issue:

  1. Run npm install.
  2. Execute npm run start.
  3. Click the button to observe the memory leak issue.

https://github.com/lucas-quinn/ionic-memory-leak

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Dec 14, 2023
@rwaskiewicz rwaskiewicz self-assigned this Dec 14, 2023
@rwaskiewicz
Copy link
Member

Hey 👋

Thanks for the issue! I’ve taken an initial look at the reproduction case you’ve provided, and it does appear there is a memory leak of some type occurring here.

Before we move forward with this, can you help us out with a few things?

Can you do me a favor and split this issue into two, one for each type of leaking described? The reason I ask is:

  • It helps us track the work associated with investigations, root cause analysis, and testing fix(es)
  • It helps us ensure that we’re not conflating details from two (potentially separate) issues of the same type

Splitting the issue can be done by creating a new issue, copying the existing details from the second reported leak into the new issue.

From there, someone on the team will take a closer look at each issue and may follow up/request some additional information.

Thanks!

@rwaskiewicz rwaskiewicz added the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Dec 14, 2023
@ionitron-bot ionitron-bot bot removed the triage label Dec 14, 2023
@Lucas-Quinn-1163273
Copy link
Author

Hey 👋

Thanks for the issue! I’ve taken an initial look at the reproduction case you’ve provided, and it does appear there is a memory leak of some type occurring here.

Before we move forward with this, can you help us out with a few things?

Can you do me a favor and split this issue into two, one for each type of leaking described? The reason I ask is:

  • It helps us track the work associated with investigations, root cause analysis, and testing fix(es)
  • It helps us ensure that we’re not conflating details from two (potentially separate) issues of the same type

Splitting the issue can be done by creating a new issue, copying the existing details from the second reported leak into the new issue.

From there, someone on the team will take a closer look at each issue and may follow up/request some additional information.

Thanks!

Thank you so much!

I split the second kind of leak into another one, #5181 . Since the second one affects production, it should be considered more important. However, this one occurs in the development environment, making it challenging to identify issues in production. Therefore, both are very important. I hope to hear from the team soon!

@ionitron-bot ionitron-bot bot removed the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Dec 14, 2023
@rwaskiewicz
Copy link
Member

Thanks @lucas-quinn !

I took the liberty of moving the list of Ionic Components to a collapsible section in the issue summary to minimize some additional scrolling, as well as adding another reference to the second issue there.

Can you do me a favor and for this issue, add a few screenshots of what you're seeing in terms of memory leaks? While I think we're on the same page, I'd like to make sure we're working on the issue/type of memory leak reported here. Thanks!

@rwaskiewicz rwaskiewicz added the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Dec 14, 2023
@Lucas-Quinn-1163273
Copy link
Author

Lucas-Quinn-1163273 commented Dec 14, 2023

@rwaskiewicz Thanks! I have added two screenshots. I'm using Microsoft Edge - Detached Elements here because Chrome doesn't offer this devtool to easily debug detached elements. But Chrome does offer memory profiling.

@ionitron-bot ionitron-bot bot removed the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Dec 14, 2023
@rwaskiewicz rwaskiewicz added the Bug: Validated This PR or Issue is verified to be a bug within Stencil label Dec 15, 2023
@rwaskiewicz rwaskiewicz removed their assignment Dec 15, 2023
@rwaskiewicz
Copy link
Member

Thanks @lucas-quinn! I've validated this bug with the reproduction case + the screenshots. I've labeled it to get it ingested into our backlog to prioritize.

@mmben
Copy link

mmben commented Apr 29, 2024

I'm not 100% sure, but I think we may be running into this leak as well. Mostly it is just some ZoneAwarePromise objects that are retained, but sometimes we have HTMLElements as well. The retainers always point to the constructor in Line 101 of bootstrap-lazy.ts:
// StencilLazyHost
constructor(self: HTMLElement) {

We're running:
@ionic/[email protected]
@ionic/[email protected]
@stencil/[email protected]
on Node 20.12.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Development

No branches or pull requests

3 participants