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

PrimeVue Tooltip: memory leak #5856

Open
csakis opened this issue Jun 9, 2024 · 7 comments
Open

PrimeVue Tooltip: memory leak #5856

csakis opened this issue Jun 9, 2024 · 7 comments
Labels
Status: Pending Review Issue or pull request is being reviewed by Core Team
Milestone

Comments

@csakis
Copy link

csakis commented Jun 9, 2024

Describe the bug

The tooltip component causes serious memory leak if the tooltip is attached to a re-rendered component. I created a minimal application to demonstrate the issue. The application was monitored with Chrome task manager. When the application started up the memory usage was this:
image
After a couple of hours running, JS memory usage increased to 44MB:
image

Reproducer

https://stackblitz.com/~/github.com/csakis/prime-test?file=vite.config.ts

PrimeVue version

3.52.0

Vue version

3.x

Language

TypeScript

Build / Runtime

Vite

Browser(s)

Chrome

Steps to reproduce the behavior

  1. Run project
  2. Monitor JS memory usage

Expected behavior

Memory usage should be constant, however, it keeps growing steadily,

@csakis csakis added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Jun 9, 2024
@DrJonki
Copy link

DrJonki commented Jun 9, 2024

Most likely a duplicate of #5629.

@csakis
Copy link
Author

csakis commented Jun 10, 2024

True, but I provided a reproducible repo.

@emp-mas
Copy link

emp-mas commented Jun 14, 2024

The Tooltip component leaks event listeners (click, keydown and mouseleave) on each execution of the updated hook.
The updated hooks calls unbindEvents() followed by bindEvents().

In bindEvents() of Tooltip.js the event listeners are created like this:
el.addEventListener('click', this.onClick.bind(this));
Function.prototype.bind() returns a new bound function, not a reference to the original function.

Therefore the removal of the event listeners in unbindEvents() using
el.removeEventListener('click', this.onClick.bind(this));
is not working. Calling this.onClick.bind(this) returns again a new function and not a reference to the handler function used in bindEvents().

Demo: https://stackblitz.com/edit/zafh9e?file=src%2FApp.vue

To be able to remove the event listeners afterwards, it's necessary to store a reference to the handler function like it's done for the mouseenter event:
el.$_mouseenterevent = (event) => this.onMouseEnter(event, options);
el.addEventListener('mouseenter', el.$_mouseenterevent);
Removal:
el.removeEventListener('mouseenter', el.$_mouseenterevent);
el.$_mouseenterevent = null;

@andersnm
Copy link

Hi, searching for more occurrences of @emp-mas 's finding, the ripple directive has the same issue

bindEvents(el) {
el.addEventListener('mousedown', this.onMouseDown.bind(this));
},
unbindEvents(el) {
el.removeEventListener('mousedown', this.onMouseDown.bind(this));
},

This affects a bunch of components, but only if enabled (...so its not the leak I'm looking for)

@ZiadJ
Copy link

ZiadJ commented Jun 14, 2024

Perhaps a WeakMap would be a better alternative. So instead of adding a property to the targeted element, it would be added to the WeakMap instead, using the element as key. The benefit over Map is that it allows the value to be garbage collected once the key is removed in unbindEvents. The downside it that it's not iteratable which is fine in this case.

@csakis
Copy link
Author

csakis commented Aug 23, 2024

is there any progress with this issue?

@KKRainbow
Copy link

KKRainbow commented Nov 10, 2024

is there any progress with this issue?
it can be reproduced on 4.2.1

@tugcekucukoglu tugcekucukoglu added this to the 4.3.1 milestone Feb 19, 2025
@github-project-automation github-project-automation bot moved this to Review in PrimeVue Feb 19, 2025
@tugcekucukoglu tugcekucukoglu added Status: Pending Review Issue or pull request is being reviewed by Core Team and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Pending Review Issue or pull request is being reviewed by Core Team
Projects
Status: Review
Development

No branches or pull requests

7 participants