[Bug] Local helpers and modifiers cause memory leaks

[bendemboski]

🐞 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:

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:

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]

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

• https://github.com/glimmerjs/glimmer-vm/blob/68d371bdccb41bc239b8f70d832e956ce6c349d8/packages/%40glimmer/manager/lib/internal/defaults.ts#L27
• which is used here: https://github.com/glimmerjs/glimmer-vm/blob/master/packages/%40glimmer/manager/lib/internal/api.ts#L153C1-L154C4

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

Upon further snooping, I found a helper definition cache

• https://github.com/glimmerjs/glimmer-vm/blob/master/packages/%40glimmer/program/lib/constants.ts#L144 (not sure if relevant tho)
• https://github.com/glimmerjs/glimmer-vm/blob/master/packages/%40glimmer/program/lib/constants.ts#L157 (This is a WeakMap)

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]

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

[Windvis]

I think this can now be closed since glimmerjs/glimmer-vm#1440 was merged and the VM update with the fix made it into Ember v5.7+.