fix(core): debug data causing memory leak for root effects

crisbeto · kirjs · commit 70507b8c1ce7 · 2025-11-24T15:26:12.000-05:00
We track all effects that are created for debugging purposes in the `resolverToEffects` map. This ends up leaking memory for effects registered on long-living resolvers (e.g. on the root injector), because they stay in the array, even if the effect itself has been destroyed. These changes add a callback to clean up the references. Fixes #65265. (cherry picked from commit ca6ab6c)
diff --git a/packages/core/src/render3/debug/framework_injector_profiler.ts b/packages/core/src/render3/debug/framework_injector_profiler.ts
@@ -18,6 +18,7 @@ import {TNode} from '../interfaces/node';
 import {LView} from '../interfaces/view';
 import {AfterRenderPhaseEffectNode} from '../reactivity/after_render_effect';
 import {EffectRefImpl} from '../reactivity/effect';
+import {SIGNAL} from '../../../primitives/signals';
 
 import {
   InjectedService,
@@ -136,12 +137,23 @@ function handleEffectCreatedEvent(
   }
 
   const {resolverToEffects} = frameworkDIDebugData;
+  const cleanupContainer = effect instanceof EffectRefImpl ? effect[SIGNAL] : effect.sequence;
+  let trackedEffects = resolverToEffects.get(diResolver);
 
-  if (!resolverToEffects.has(diResolver)) {
-    resolverToEffects.set(diResolver, []);
+  if (!trackedEffects) {
+    trackedEffects = [];
+    resolverToEffects.set(diResolver, trackedEffects);
   }
 
-  resolverToEffects.get(diResolver)!.push(effect);
+  trackedEffects.push(effect);
+  cleanupContainer.onDestroyFns ??= [];
+  cleanupContainer.onDestroyFns.push(() => {
+    const index = trackedEffects!.indexOf(effect);
+
+    if (index > -1) {
+      trackedEffects!.splice(index, 1);
+    }
+  });
 }
 
 /**
diff --git a/packages/core/src/render3/reactivity/after_render_effect.ts b/packages/core/src/render3/reactivity/after_render_effect.ts
@@ -177,6 +177,9 @@ export class AfterRenderEffectSequence extends AfterRenderSequence {
     AfterRenderPhaseEffectNode | undefined,
   ] = [undefined, undefined, undefined, undefined];
 
+  /** Function to be called when the effect is destroyed. */
+  onDestroyFns: (() => void)[] | null = null;
+
   constructor(
     impl: AfterRenderImpl,
     effectHooks: Array<AfterRenderPhaseEffectHook | undefined>,
@@ -234,6 +237,12 @@ export class AfterRenderEffectSequence extends AfterRenderSequence {
   }
 
   override destroy(): void {
+    if (this.onDestroyFns !== null) {
+      for (const fn of this.onDestroyFns) {
+        fn();
+      }
+    }
+
     super.destroy();
 
     // Run the cleanup functions for each node.
diff --git a/packages/core/src/render3/reactivity/effect.ts b/packages/core/src/render3/reactivity/effect.ts
@@ -23,7 +23,6 @@ import {assertNotInReactiveContext} from './asserts';
 import {assertInInjectionContext} from '../../di/contextual';
 import {DestroyRef, NodeInjectorDestroyRef} from '../../linker/destroy_ref';
 import {ViewContext} from '../view_context';
-import {noop} from '../../util/noop';
 import {
   ChangeDetectionScheduler,
   NotificationSource,
@@ -177,7 +176,7 @@ export function effect(
 
   if (destroyRef !== null) {
     // If we need to register for cleanup, do that here.
-    node.onDestroyFn = destroyRef.onDestroy(() => node.destroy());
+    node.onDestroyFns = [destroyRef.onDestroy(() => node.destroy())];
   }
 
   const effectRef = new EffectRefImpl(node);
@@ -200,7 +199,7 @@ export interface EffectNode extends BaseEffectNode, SchedulableEffect {
   injector: Injector;
   notifier: ChangeDetectionScheduler;
 
-  onDestroyFn: () => void;
+  onDestroyFns: (() => void)[] | null;
 }
 
 export interface ViewEffectNode extends EffectNode {
@@ -216,7 +215,7 @@ export const EFFECT_NODE: Omit<EffectNode, 'fn' | 'destroy' | 'injector' | 'noti
     ...BASE_EFFECT_NODE,
     cleanupFns: undefined,
     zone: null,
-    onDestroyFn: noop,
+    onDestroyFns: null,
     run(this: EffectNode): void {
       if (ngDevMode && isInNotificationPhase()) {
         throw new Error(`Schedulers cannot synchronously execute watches while scheduling.`);
@@ -259,7 +258,13 @@ export const ROOT_EFFECT_NODE: Omit<RootEffectNode, 'fn' | 'scheduler' | 'notifi
     },
     destroy(this: RootEffectNode) {
       consumerDestroy(this);
-      this.onDestroyFn();
+
+      if (this.onDestroyFns !== null) {
+        for (const fn of this.onDestroyFns) {
+          fn();
+        }
+      }
+
       this.cleanup();
       this.scheduler.remove(this);
     },
@@ -275,7 +280,13 @@ export const VIEW_EFFECT_NODE: Omit<ViewEffectNode, 'fn' | 'view' | 'injector' |
     },
     destroy(this: ViewEffectNode): void {
       consumerDestroy(this);
-      this.onDestroyFn();
+
+      if (this.onDestroyFns !== null) {
+        for (const fn of this.onDestroyFns) {
+          fn();
+        }
+      }
+
       this.cleanup();
       this.view[EFFECTS]?.delete(this);
     },
diff --git a/packages/core/test/acceptance/signal_debug_spec.ts b/packages/core/test/acceptance/signal_debug_spec.ts
@@ -17,6 +17,8 @@ import {
   Injectable,
   signal,
   Injector,
+  ApplicationRef,
+  afterRenderEffect,
 } from '../../src/core';
 import {
   getFrameworkDIDebugData,
@@ -445,4 +447,38 @@ describe('getSignalGraph', () => {
     expect(edgesWithNodes).toContain({consumer: effectBNode!, producer: signalANode!});
     expect(edgesWithNodes).toContain({consumer: effectDNode!, producer: signalCNode!});
   });
+
+  it('should stop tracking effect when ref is destroyed', () => {
+    @Component({template: ''})
+    class App {}
+
+    const fixture = TestBed.createComponent(App);
+    fixture.detectChanges();
+
+    const injector = TestBed.inject(ApplicationRef).injector;
+    expect(getFrameworkDIDebugData().resolverToEffects.has(injector)).toBe(false);
+
+    const ref = effect(() => {}, {injector});
+    expect(getFrameworkDIDebugData().resolverToEffects.get(injector)?.length).toBe(1);
+
+    ref.destroy();
+    expect(getFrameworkDIDebugData().resolverToEffects.get(injector)?.length).toBe(0);
+  });
+
+  it('should stop tracking afterRenderEffect when ref is destroyed', () => {
+    @Component({template: ''})
+    class App {}
+
+    const fixture = TestBed.createComponent(App);
+    fixture.detectChanges();
+
+    const injector = TestBed.inject(ApplicationRef).injector;
+    expect(getFrameworkDIDebugData().resolverToEffects.has(injector)).toBe(false);
+
+    const ref = afterRenderEffect(() => {}, {injector});
+    expect(getFrameworkDIDebugData().resolverToEffects.get(injector)?.length).toBe(1);
+
+    ref.destroy();
+    expect(getFrameworkDIDebugData().resolverToEffects.get(injector)?.length).toBe(0);
+  });
 });
