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] Local helpers and modifiers cause memory leaks #20535

Open
bendemboski opened this issue Sep 1, 2023 · 2 comments
Open

[Bug] Local helpers and modifiers cause memory leaks #20535

bendemboski opened this issue Sep 1, 2023 · 2 comments

Comments

@bendemboski
Copy link
Contributor

🐞 Describe the Bug

Anytime a component that has a local modifier or helper is rendered, that modifier/helper object/function will be retained in memory even after the component is un-rendered (it is released when the app exits). The fact that it is retained after the component is un-rendered is why I'm calling it a memory leak.

This has two implications that, especially when considered together, can cause quite a lot of memory to leak:

  1. If the helper/modifier is one-per-instance, e.g. readonly myHelper = function myHelper() { return 'hello'; }, then a copy of the myHelper function will leak for each component instance that is rendered/un-rendered
  2. If the helper/modifier closes over the component instance (e.g. an arrow function that references this), then the component instance itself will leak

🔬 Minimal Reproduction

This repository contains a minimal reproduction of the issue, both via an integration test, and steps for a manual repro in the dummy app.

😕 Actual Behavior

After rendering and un-rendering a component that invokes a local helper, that local helper function leaks.

🤔 Expected Behavior

It should be released when the component is un-rendered.

🌍 Environment

  • Ember: 5.2.0
  • Node.js/npm: 16.15.1
  • OS: MacOS
  • Browser: Chrome/Firefox/Safari

➕ Additional Context

I dug in some and believe I have a diagnosis of the problem. Here is the culprit retainer chain:

MemoryLeak

The CompileTimeCompilationContextImpl is effectively a singleton, as is ConstantsImpl. When the local helper is first invoked, ConstantsImpl's helper() method is invoked, which calls into value(), which puts an object containing the helper arrow function in a this.values and this.indexMap:

MemoryLeak

They are not removed until the app exits.

This is some guesswork, but it appears that these helpers are being stored in the same place as global helpers, so the pattern is to keep them cached forever, but really they should be stored in a place/manner such that their lifetime is bound to the lifetime of the component, and/or they should stored via weak references.

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Sep 1, 2023

I haven't looked in to this too deeply yet, but here is the manager for plain functions:

I wonder if the manager infra is holding on to the function reference

Upon further snooping, I found a helper definition cache

idk if you want to dig in to glimmer, but resolving any sort of memory leak would be a huge help <3 💪

as an aside, how do find out what is still holding on to a reference?

@bendemboski
Copy link
Contributor Author

My stab at a fix: glimmerjs/glimmer-vm#1440

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

No branches or pull requests

3 participants