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

Potential Memory Leak Issue in ngrx Store-Component #3982

Closed
1 of 2 tasks
itzikbuktshin opened this issue Jul 31, 2023 · 1 comment · Fixed by #4001
Closed
1 of 2 tasks

Potential Memory Leak Issue in ngrx Store-Component #3982

itzikbuktshin opened this issue Jul 31, 2023 · 1 comment · Fixed by #4001
Labels
Accepting PRs Comp: Docs Good First Issue Good issue for first-time contributor

Comments

@itzikbuktshin
Copy link

Information

I am writing to bring to your attention a potential memory leak issue that I discovered in the ngrx store component.
As an avid user of the library, I noticed some behaviors that suggest there might be a problem when overriding the ngOnDestroy method,
where the parent class does not release resources properly.

Description of the Issue:
Upon thorough analysis, I suspect that the store component may be causing a memory leak when the ngOnDestroy method is overridden. The parent class will not be called, and as a result and resources will not release, which may lead to inefficient memory utilization.

Expected Behavior:
When overriding the onDestory you should add super.onDestory()

Relevant Code Snippet:

/** Completes all relevant Observable streams. */ ngOnDestroy() { this.stateSubject$.complete(); this.destroySubject$.next(); }

Impact:
A memory leak issue can severely impact the performance and stability of applications using the ngrx store-component. It can lead to increased memory consumption, decreased performance, and even application crashes in severe cases.

Documentation page

https://v15.ngrx.io/guide/component-store/lifecycle#ondestroy

I would be willing to submit a PR to fix this issue

  • Yes
  • No
@timdeschryver
Copy link
Member

This can indeed result in a leak.
We can add the following snippet/warning in the docs when a class extends from a component store.

  override ngOnDestroy(): void {
    // 👇 add this line
    super.ngOnDestroy();
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepting PRs Comp: Docs Good First Issue Good issue for first-time contributor
Projects
None yet
2 participants