[BUGFIX] clear stale metadata references after destruction to allow GC

[Copilot]

After destroy() completes, DESTROYABLE_META entries retained strong references to parents, children, eagerDestructors, and destructors, forming reference cycles through the WeakMap values that prevented GC from reclaiming destroyed object trees. In large test suites (e.g. Discourse's ~5,000-test suite) this caused OOM on CI.

Two issues combine to cause this:

1. scheduleDestroyed only sets meta.state = DESTROYED_STATE — never nulls the other fields
2. removeChildFromParent guards on parentMeta.state === LIVE_STATE, so during cascaded destruction the child's callback skips removing itself from the parent (parent is already DESTROYING_STATE), leaving parent.meta.children holding a stale reference

Changes

• packages/@glimmer/destroyable/index.ts — null out all metadata references in scheduleDestroyed after DESTROYED_STATE is set:

  scheduleDestroyed(() => {
    iterate(parents, (parent) => {
      removeChildFromParent(destroyable, parent);
    });
  
    meta.state = DESTROYED_STATE;
  
    // Release references so GC can reclaim the destroyed tree
    meta.parents = null;
    meta.children = null;
    meta.eagerDestructors = null;
    meta.destructors = null;
  });

  Safe because once DESTROYED_STATE is set, destroy() returns early and no other API reads these fields.

• packages/@glimmer/destroyable/test/destroyables-test.ts — adds a regression test asserting _hasDestroyableChildren(parent) returns false after destroy(parent) + flush(), which fails without the fix since the stale child reference lingers in meta.children.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>@glimmer/destroyable: destroy() retains stale references in metadata after destruction, delaying GC of parent-child trees</issue_title>
<issue_description>### 🐞 Describe the Bug

While upgrading Discourse from Ember 6.6 to 6.10, our QUnit test suite (~5,000 tests) started running out of memory in Chrome on CI. Through bisecting ember-source versions we traced the regression to Ember 6.8, where the renderer refactoring added multiple new associateDestroyableChild() calls, creating deeper destroyable ownership trees.

The underlying issue is in @glimmer/destroyable's destroy() function: after destruction completes, the metadata entries in DESTROYABLE_META retain stale object references, preventing the garbage collector from reclaiming the destroyed object tree.

Two issues combine to cause this:

1. The scheduleDestroyed callback sets meta.state = DESTROYED_STATE but never clears the parents, children, eagerDestructors, or destructors properties.

2. removeChildFromParent has a parentMeta.state === LIVE_STATE guard that checks the parent's state. During cascaded destruction (destroy(parent) → destroy(child)), the child's scheduleDestroyed callback is queued before the parent's. When it runs and calls removeChildFromParent(child, parent), the parent's state is already DESTROYING_STATE (set synchronously at the start of destroy(parent)), so the guard skips the removal.

After destroy(parent) completes:

• The parent's metadata still holds strong references to all children via meta.children
• Each child's metadata still holds a strong reference back to the parent via meta.parents
• These cross-references create cycles through the WeakMap values: each entry's value (the metadata) holds a strong reference to another entry's key (the related destroyable), keeping the entire graph alive

In workloads that rapidly create and destroy large destroyable trees (such as test suites that spin up an Application per test), these stale references put significant pressure on the garbage collector, causing memory to accumulate and eventually leading to OOM.

🔬 Minimal Reproduction

The core issue can be illustrated with this pattern:

import { associateDestroyableChild, destroy } from '@ember/destroyable';

// Create a parent-child tree
const parent = {};
const child = {};
associateDestroyableChild(parent, child);

destroy(parent);

// After the scheduleDestroyed callbacks run:
// - DESTROYABLE_META.get(parent).children still references child
// - DESTROYABLE_META.get(child).parents still references parent
// - Both entries are in DESTROYED_STATE but hold strong cross-references
// - These cross-references through the WeakMap keep the entire tree alive

In Discourse's QUnit test suite (~5,000 tests), each test creates and destroys an Application instance. Even with explicit app.destroy() and releasing the app reference, Chrome runs out of memory on CI. Adding the metadata cleanup in the scheduleDestroyed callback (see proposed fix below) resolves the OOM completely.

😕 Actual Behavior

After destroy() completes and meta.state reaches DESTROYED_STATE, the metadata retains all parents, children, eagerDestructors, and destructors references. Combined with the LIVE_STATE guard in removeChildFromParent, this means:

1. Parent's meta.children still references the child — removeChildFromParent in the child's scheduleDestroyed callback checks parentMeta.state === LIVE_STATE, but the parent is already in DESTROYING_STATE by that point, so the removal is skipped
2. Child's meta.parents still references the parent — no code path clears this after destruction
3. The DESTROYABLE_META WeakMap entries form reference cycles through their values, keeping the entire destroyed tree alive

🤔 Expected Behavior

After destruction completes (meta.state = DESTROYED_STATE), the metadata should eagerly release all object references so the garbage collector can reclaim the destroyed tree sooner.

Proposed fix in @glimmer/destroyable:

scheduleDestroyed(() => {
  iterate(parents, (parent) => {
    removeChildFromParent(destroyable, parent);
  });

  meta.state = DESTROYED_STATE;

  // Release references so GC can reclaim the destroyed tree
  meta.parents = null;
  meta.childr...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes emberjs/ember.js#21145

<!-- START COPILOT CODING AGENT TIPS -->
---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security)

[github-actions]

📊 Package size report   -0%↓

File	Before (Size / Brotli)	After (Size / Brotli)
Total (Includes all files)	17.1 MB / 3.2 MB	-0%↓17.1 MB / 0.01%↑3.2 MB
Tarball size	3.8 MB	-0%↓3.8 MB

_{🤖 This report was automatically generated by pkg-size-action}

[ef4]

This isn't correct. The meta itself should also be released. It's supposed to be released because it's stored in a WeakMap.

I see that there is a debugging feature that replaces the WeakMap with a Map, but in that case you're supposed to managed paired calls to enableDestroyableTracking and assertDestroyablesDestroyed, which will eventually replace the Map again with a new WeakMap.

So either something is using the debug feature and not pairing those calls correctly, or (more likely) there's something elsewhere retaining the objects that meta is associated with.

[NullVoxPopuli]

@copilot see ef4's comment <3

[NullVoxPopuli]

This new change is way more targeted. 🤔

Anyone know how to test for memory leaks in qunit? 🤔

[NullVoxPopuli]

Here is a PR with a patch of this fix: discourse/discourse#38395

thanks @megothss !!!