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.

Sign up for GitHub

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

Jump to bottom

potential memory leak #1717

Open
2 tasks done
piotretm opened this issue Feb 28, 2025 · 6 comments
Open
2 tasks done

potential memory leak #1717

piotretm opened this issue Feb 28, 2025 · 6 comments
Labels
triage We discuss this topic in our internal weekly

Comments

@piotretm
Copy link

Prerequisites

  • I have read the Contributing Guidelines.
  • I have not leaked any internal/restricted information like screenshots, videos, code snippets, links etc.

What happened?

During the investigation in our application, we've noticed that some of your components might be causing memory leaks.

First, removing the ix-drawer reduced the heap size in our application. When we use the component, it stays retained.

Second, we've noticed that the dropdown-controller and components that utilize it (date-input and tree) might also suffer from not removed event listeners and cause memory leaks (events are listened to in this function).

In both cases, it is related to the fact that event listeners are not removed - either because there is no code for it at all or it is not reached before the component is removed from the DOM tree.

Please take a look at those components but also other places where the clean up might not be done properly.

I provide a code snippet based on a stackblitz from the docs of one of your components to test the issue a bit. You can add/remove components and observe the memory consumption by taking heap snapshots or turning on the memory monitor in Chrome DevTools.

What type of frontend framework are you seeing the problem on?

Others, JavaScript

Which version of iX do you use?

v2.7.0

Code to produce this issue.

// copy-paste of the code from snippet
// HTML
<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Date input example</title>
  </head>
  <body>
    <button id="run">CREATE</button>
    <button id="remove">REMOVE</button>
    <div></div>
    <script type="module" src="./init.js"></script>
  </body>
</html>
// JavaScript - init.js file
import './styles/global.css';
import { defineCustomElements } from '@siemens/ix/loader';
import { defineCustomElements as defineIconsCustomElements } from '@siemens/ix-icons/loader';

defineIconsCustomElements();
defineCustomElements();

function remove() {
  const container = document.querySelector('div');
  container.innerHTML = ''; // Remove all elements
}

// Step 1: Dynamically create and remove date inputs
function createelements(count = 100) {
  const container = document.querySelector('div');

  for (let i = 0; i < count; i++) {
    console.log('create');
    const component = document.createElement('ix-drawer');
    // const component = document.createElement('ix-date-input');
    component.innerText = 'TEST';
    component.setAttribute('show', 'true');
    container.appendChild(component);
  }
}

const btn = document.querySelector('#run');
btn.addEventListener('click', () => {
  createelements();
});

const removeBtn = document.querySelector('#remove');
removeBtn.addEventListener('click', () => {
  remove();
});
@piotretm piotretm added the triage We discuss this topic in our internal weekly label Feb 28, 2025
@danielleroux
Copy link
Collaborator

danielleroux commented Feb 28, 2025 •
edited
Loading

Can you give us more insights into your observations, I tested your snippet and did some memory checks:

Drawer:
Image

DateInput:
Image

Looks like the GC cleans up the heap.

Dropdown Controller:
I checked the linked source code and there is an boolean which prevents creating multiple event-listener for an dropdown-controller instance.

ix/packages/core/src/components/dropdown/dropdown-controller.ts

Lines 57 to 59 in 699958d

if (!this.isWindowListenerActive) {
this.addOverlayListeners();
}

ix/packages/core/src/components/dropdown/dropdown-controller.ts

Lines 204 to 207 in 699958d

private addOverlayListeners() {
this.isWindowListenerActive = true;
window.addEventListener('click', (event: MouseEvent) => {

Also checked the browser instance there is event after some add/remove element still only one event-listener registered.
Image

So any further details would be helpful to solve the issue as fast as possible.

@piotretm
Copy link
Author

Regarding the dropdown controller, shouldn't the listeners be removed anyway if the component that was using them (for instance, the date input) was removed from the DOM tree? Otherwise, it might block the element from being garbage collected if I get this right.

Regarding more insight, I can explain what I was doing in the provided code snippet.

  • open memory tab in DevTools and take a heap snapshot
    Image
  • click create button and take a snapshot
    Image
  • click remove button and take a snapshot
    Image

As you can see, even though the elements were removed they are still in the memory. Doing this multiple times will increase the number of detached elements. This is a result after doing the create/remove steps 2 more times
Image

If you try to do the same with a different component (I tried ix-pill) this is not reproducable.

@danielleroux
Copy link
Collaborator

danielleroux commented Feb 28, 2025 •
edited
Loading

can you repeat your check using the @siemens/ix/components/ix-drawer.js and @siemens/ix/components/ix-date-input.js or
use a react runtime, there is a known WeakMap reference in stenciljs but only in the lazy loading part (@siemens/ix/loader).

Regarding the dropdown controller, shouldn't the listeners be removed anyway if the component that was using them (for instance, the date input) was removed from the DOM tree?

No component holding a reference its just the singleton from the controller itself.

As a alternative you can try to update (overwrite) @stencil/core to the version 4.27.1

@piotretm
Copy link
Author

mmm, I tried it on the react stackblitz for ix-drawer and seems like issue is there as well.
Image
Difference in size is smaller, because I was adding and removing a single element.

@danielleroux
Copy link
Collaborator

Can you try to update to one of these version:

Core library:

npm i @siemens/[email protected]

Angular:

npm i @siemens/[email protected]

React:

npm i @siemens/[email protected]

Vue:

npm i @siemens/[email protected]

@piotretm
Copy link
Author

I've tested using stackblitz for react and core (with loaders). In both cases the issue still occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
triage We discuss this topic in our internal weekly
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@danielleroux @piotretm