Skip to content

fix: memory leak in element-controller#7684

Description

@sm3rta

馃悰 Bug Report

In fast-element v3, forCustomElement() now subscribes two closures to Observable.getNotifier(definition): one for template and one for shadowOptions.

Both closures capture the element, and never unsubscribe. Because a FASTElementDefinition is a per-tag singleton that lives as long as the page, every element instance was reachable from its definition forever. Removing an element from the DOM and dropping all user references was not enough to make it garbage collectable.

The same path also compounds. The handler calls forCustomElement(element, true), which re-enters the subscribe calls, so a single element's subscription count doubles on every template swap (measured 1 -> 2 -> 4 -> 8 -> 16).

This is new in v3. In v2.0.0-beta.26, forCustomElement is six lines with no subscriptions and no override parameter, so both the retention and the doubling were introduced with v3.

馃捇 Repro or Code Sample

Run Chrome with --js-flags="--expose-gc" to enable forced garbage collection:

import { FASTElement, html } from '@microsoft/fast-element';

class LeakTest extends FASTElement {}
LeakTest.define({ name: 'leak-test', template: html`<span>hello</span>` });

const host = document.createElement('div');
document.body.appendChild(host);

const refs = [];
for (let i = 0; i < 300; i++) {
    const el = document.createElement('leak-test');
    host.appendChild(el);
    refs.push(new WeakRef(el));
}

host.replaceChildren();
await new Promise(resolve => setTimeout(resolve, 100));
globalThis.gc();

console.log(refs.filter(ref => ref.deref()).length); // 300

A plain div or a native button in the same harness reports 0.

There is also a deterministic check that needs no GC: the subscriber count on the definition's notifier grows 1:1 with instances created (250 -> 251, 500 -> 502, 1000 -> 1004) on both the template and shadowOptions lists.

馃 Expected Behavior

An element that has been detached from the DOM, with no remaining user references, should be garbage collectable. The count logged above should be 0, and repeated create/destroy cycles should not grow the heap.

馃槸 Current Behavior

Every element survives collection and the heap grows linearly without bound.

Measured in Chrome after forced GC, creating and destroying elements in cycles:

Build Retained after forced GC Heap growth
v3 (3.0.2) 300/300 11.50 MB/cycle
v2 (2.0.0-beta.26, used as control) 1/300 0.14 MB/cycle

A heap snapshot diff over 5 cycles x 500 elements shows +2500 elements, +2500 ElementControllers, and +5000 closure:handleChange: exactly two per element, matching the two subscriptions.

Per-element retention cost, measured under happy-dom (relative figures, not browser-accurate):

Scenario Per element
No template ~7.0 KB
Light DOM ~22.4 KB
Shadow DOM ~29.9 KB

馃拋 Possible Solution

I have a working fix (element-controller-leak.patch) and I'd be happy to contribute it:

  • Register one subscription per definition instead of one per element, iterating a Set<WeakRef<FASTElement>> and pruning dead references as it goes, so the definition no longer strongly retains its elements.
  • Drop the shadowOptions subscription: shadowOptions is not Observable.defineProperty'd on FASTElementDefinition, so assigning it produces zero notifications anyway 馃

With the fix: 1/300 retained, 0.18 MB/cycle, matching the leak-free v2 control. All 1468 fast-element Chromium tests pass, and api-extractor reports no API report diff.

Note about proposed fix

Weigh in needed on the change in tsconfig.json:

This fix uses WeakRefs, so PR can't be merged as is without accepting the raised runtime floor.

If upgrading to es2021 is out of the question, an alternative approach would be to unsubscribe on teardown: when a component is disconnected, its subscriptions are removed. Happy to switch if you prefer it!

馃敠 Context

I accidentally hit this while benchmarking fast-element based components by rendering and removing them repeatedly. Memory climbed until the Node SSR process died of heap exhaustion (~700 MB per render, even with an explicit global.gc() after each one).

It affects any application that repeatedly creates and destroys components (i.e. SPA navigation, virtualized lists, dialogs) where the retention accumulates over a session rather than being reclaimed.

馃實 Your Environment

  • OS & Device: Windows 11 Enterprise (10.0.26100) on PC
  • Browser: Google Chrome 151.0.7922.174
  • Node: v22.14.0, npm 10.9.2
  • Version: @microsoft/fast-element 3.0.2

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions