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

All components: Memory leak #6715

Open
YannisJustine opened this issue Nov 2, 2024 · 3 comments · May be fixed by #6889
Open

All components: Memory leak #6715

YannisJustine opened this issue Nov 2, 2024 · 3 comments · May be fixed by #6889
Labels
Status: Pending Review Issue or pull request is being reviewed by Core Team
Milestone

Comments

@YannisJustine
Copy link
Contributor

YannisJustine commented Nov 2, 2024

Describe the bug

Memory leak : callbacks in EventBus are never removed

Description

A memory leak issue exists due to the EventBus. Some callback functions registered to the EventBus are never removed, causing unused callbacks to accumulate in memory over time. This leads to increased memory usage, especially in applications where components with registered callbacks are frequently added and removed.

Steps to Reproduce

  1. Go to official website
  2. Navigate to a component page.
  3. Switch to another component page.
  4. Repeat steps 2 and 3 several times.

Before

image

After

image

Expected Behavior

When a component is destroyed or unmounted, the associated callbacks registered to the EventBus should be removed automatically.

Actual Behavior

Callbacks registered with EventBus.on() are never removed, even after the component is unmounted. This causes memory leaks.

Examples of Affected Code

The following code samples demonstrate instances within PrimeVue where callbacks are registered to EventBus without being properly removed, contributing to memory leaks:

BaseDirective.js

In the BaseDirective.js file, the following code snippet shows how config and config.ripple watchers register listeners on PrimeVueService without cleanup:

const handleWatch = (el) => {
    const watchers = el.$instance?.watch;

    // for 'config'
    watchers?.['config']?.call(el.$instance, el.$instance?.$primevueConfig);
    PrimeVueService.on('config:change', ({ newValue, oldValue }) => watchers?.['config']?.call(el.$instance, newValue, oldValue));

    // for 'config.ripple'
    watchers?.['config.ripple']?.call(el.$instance, el.$instance?.$primevueConfig?.ripple);
    PrimeVueService.on('config:ripple:change', ({ newValue, oldValue }) => watchers?.['config.ripple']?.call(el.$instance, newValue, oldValue));
};

Similarly, _loadStyles registers listeners for theme changes but does not remove them:

_loadStyles: (el, binding, vnode) => {
    const config = BaseDirective._getConfig(binding, vnode);
    const useStyleOptions = { nonce: config?.csp?.nonce };

    BaseDirective._loadCoreStyles(el.$instance, useStyleOptions);
    BaseDirective._loadThemeStyles(el.$instance, useStyleOptions);
    BaseDirective._loadScopedThemeStyles(el.$instance, useStyleOptions);

    BaseDirective._themeChangeListener(() => BaseDirective._loadThemeStyles(el.$instance, useStyleOptions));
};

BaseComponent.vue

In BaseComponent.vue, the dt watcher registers a _themeChangeListener that is never removed:

dt: {
    immediate: true,
    handler(newValue) {
        if (newValue) {
            this._loadScopedThemeStyles(newValue);
            this._themeChangeListener(() => this._loadScopedThemeStyles(newValue));
        } else {
            this._unloadScopedThemeStyles();
        }
    }
}

There are additional instances throughout the code with similar issues, where listeners are registered but not removed after component unmounting.

Proposed Solution

Remove callbacks from EventBus when the component is unmounted or destroyed.

This issue is a generalization of the memory leak reported in issue #6223

Reproducer

https://primevue.org

PrimeVue version

4.2.0

Vue version

4.x

Language

TypeScript

Build / Runtime

Vue CLI App

Browser(s)

No response

@YannisJustine YannisJustine added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Nov 2, 2024
@raizdev
Copy link

raizdev commented Nov 2, 2024

https://github.com/primefaces/primeuix/blob/main/packages/utils/src/eventbus/index.ts#L35

using slice and map together is slow...

it will not affect the memory leak but this could be better so use handlers.forEach((handler) => handler(evt)); instead.

@mertsincan
Copy link
Member

Thanks a lot for your report! I'll check and get back to you.

@mertsincan mertsincan 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 Nov 2, 2024
@mertsincan mertsincan added this to the 4.2.2 milestone Nov 2, 2024
@is-paranoia
Copy link
Contributor

I'm seeing the same problem. Version 4.0.7

Removing PrimeVue components causes the leak to disappear or become negligible

@tugcekucukoglu tugcekucukoglu modified the milestones: 4.2.2, 4.2.3 Nov 14, 2024
@tugcekucukoglu tugcekucukoglu modified the milestones: 4.2.3, 4.2.4 Nov 22, 2024
@tugcekucukoglu tugcekucukoglu modified the milestones: 4.2.4, 4.2.5 Nov 27, 2024
@YannisJustine YannisJustine linked a pull request Nov 28, 2024 that will close this issue
@tugcekucukoglu tugcekucukoglu modified the milestones: 4.2.5, 4.2.6 Dec 12, 2024
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: No status
Development

Successfully merging a pull request may close this issue.

5 participants