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] Memory Leak in RouterService + rendering tests #19684

Closed
hmajoros opened this issue Jul 30, 2021 · 1 comment · Fixed by #19685
Closed

[Bug] Memory Leak in RouterService + rendering tests #19684

hmajoros opened this issue Jul 30, 2021 · 1 comment · Fixed by #19685

Comments

@hmajoros
Copy link

🐞 Describe the Bug

In any rendering test which uses the router on ember 3.24, ember's RouterService is retaining an instance of the EmberRouter class, and is holding on to one instance per test module. in large apps, this can cause the browser to crash while running the entire test suite due to browser OOM.

This change seems to have been introduced in this PR. The RouterService caches the EmberRouter and never cleans up the reference.

🔬 Minimal Reproduction

  • Running an app using ember 3.24+, create a basic rendering test which uses the router service.
  • Using Chrome's Memory tab, run the test and capture a heap dump
  • Inspect the heap dump and notice instance of PrivateRouter class

😕 Actual Behavior

RouterService is hanging on to a reference to EmberRouter.

🤔 Expected Behavior

References to EmberRouter should be cleaned up in between tests.

🌍 Environment

  • Ember: 3.24.4 or higher
  • Node.js/npm: v14.15.1
  • OS: Mac
  • Browser: Chrome

➕ Additional Context

N/A

@hmajoros
Copy link
Author

Proposed fix would be adding a willDestroy hook in the RouterService that essentially just does:

willDestroy() {
  super.willDestroy(...arguments);
  this[ROUTER] = null;
}

I tested this fix locally against my codebase and it resolved the memory leak.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant