From 660cd8c68ea48d615fad9a13f814fa8e5a7309cf Mon Sep 17 00:00:00 2001 From: Stewart McGown Date: Sat, 11 Apr 2026 00:36:20 +0100 Subject: [PATCH 1/3] fix(core): prevent dangling prevConsumer reference from leaking destroyed views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `producerAccessed` creates a new link for a non-live consumer (e.g. a computed signal with no readers), it eagerly sets `prevConsumer` to the producer's current `consumersTail`. However, because the consumer is not live, `producerAddLiveConsumer` is skipped and the link is never inserted into the producer's consumer doubly-linked list. This means the link holds a reference *into* the producer's consumer list without being *part* of it. When the node that `prevConsumer` points to is later removed via `producerRemoveLiveConsumerLink`, the dangling link is not patched because it isn't traversable from the list. The result is that the removed consumer link — and everything it references — is kept alive by the dangling `prevConsumer` pointer on the non-live link, which itself is kept alive through the computed signal's `producers` linked list. In practice this causes multi-MB memory leaks in Angular apps: a root-provided service with a computed signal (e.g. `AttachmentApiService.urls`) holds a producer link to `ApplicationEnvironmentService.environmentSignal`. That link's `prevConsumer` captures a stale reference to a destroyed view's `ReactiveLViewConsumer` link, retaining the entire LView hierarchy — components, QueryLists, ElementRefs, and detached DOM — after the view is destroyed. The fix initializes `prevConsumer` to `undefined` at link creation time. This is safe because `producerAddLiveConsumer` unconditionally sets `link.prevConsumer = consumersTail` (line 513) when the link is actually inserted into the consumer list. The value set in `producerAccessed` was always overwritten for live consumers, and was never correct for non-live consumers. Made-with: Cursor --- packages/core/primitives/signals/src/graph.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/primitives/signals/src/graph.ts b/packages/core/primitives/signals/src/graph.ts index 144105b4c569..fd64bc226d58 100644 --- a/packages/core/primitives/signals/src/graph.ts +++ b/packages/core/primitives/signals/src/graph.ts @@ -266,7 +266,11 @@ export function producerAccessed(node: ReactiveNode): void { // instead of eagerly destroying the previous link, we delay until we've finished recomputing // the producers list, so that we can destroy all of the old links at once. nextProducer: nextProducerLink, - prevConsumer: prevConsumerLink, + // Don't set prevConsumer here — it's only meaningful when the link is part of + // the producer's consumer list. producerAddLiveConsumer sets it correctly when + // the link is actually inserted. Setting it eagerly would create a dangling + // reference into the consumer list that prevents GC of removed entries. + prevConsumer: undefined, lastReadVersion: node.version, nextConsumer: undefined, }; From 84a0f408b26ec9b521baeddd9e4fa7c2676e5575 Mon Sep 17 00:00:00 2001 From: Stewart McGown Date: Sat, 11 Apr 2026 00:52:09 +0100 Subject: [PATCH 2/3] test(core): add GC-based tests for signal graph prevConsumer leak fix Uses WeakRef + global.gc() to verify that destroyed effect consumers become garbage-collectable when a non-live computed reads the same producer. The jasmine_test target is configured with node_options: --expose-gc. GC tests are skipped in browser targets via isBrowser from @angular/private/testing. Made-with: Cursor --- packages/core/test/signals/BUILD.bazel | 2 + .../test/signals/signal_graph_leak_spec.ts | 77 +++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 packages/core/test/signals/signal_graph_leak_spec.ts diff --git a/packages/core/test/signals/BUILD.bazel b/packages/core/test/signals/BUILD.bazel index 93648675e3a7..d3602b0b55d1 100644 --- a/packages/core/test/signals/BUILD.bazel +++ b/packages/core/test/signals/BUILD.bazel @@ -12,6 +12,7 @@ ts_project( "//packages/core", "//packages/core/primitives/signals", "//packages/core/src/util", + "//packages/private/testing", ], ) @@ -20,6 +21,7 @@ jasmine_test( data = [ ":signals_lib", ], + node_options = ["--expose-gc"], ) ng_web_test_suite( diff --git a/packages/core/test/signals/signal_graph_leak_spec.ts b/packages/core/test/signals/signal_graph_leak_spec.ts new file mode 100644 index 000000000000..ac22a3f8f202 --- /dev/null +++ b/packages/core/test/signals/signal_graph_leak_spec.ts @@ -0,0 +1,77 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import {computed, signal, WritableSignal} from '../../src/core'; +import {ReactiveNode, SIGNAL} from '../../primitives/signals'; +import {isBrowser, timeout} from '@angular/private/testing'; + +import {flushEffects, resetEffects, testingEffect} from './effect_util'; + +describe('signal graph: destroyed consumers should be GC-eligible', () => { + if (isBrowser) { + it('should pass', () => {}); + return; + } + + afterEach(() => { + resetEffects(); + }); + + function setupAndReturnRef(source: WritableSignal): WeakRef { + const destroyEffect = testingEffect(() => source()); + flushEffects(); + + const sourceNode = source[SIGNAL] as ReactiveNode; + let watchNode; + let link = (sourceNode as any).consumers; + while (link !== undefined) { + watchNode = link.consumer; + link = link.nextConsumer; + } + const ref = new WeakRef(watchNode!); + + // Non-live computed that also reads source. + const derived = computed(() => source() + 1); + derived(); + + // Clearing the graph edges. + destroyEffect(); + + return ref; + } + + it('should GC a destroyed effect when a non-live computed reads the same producer', async () => { + const source = signal(0); + const ref = setupAndReturnRef(source); + + (globalThis as any).gc(); + await timeout(); + (globalThis as any).gc(); + + expect(ref.deref()).toBeUndefined(); + }); + + it('should GC destroyed effects across repeated create/destroy cycles', async () => { + const source = signal(0); + const derived = computed(() => source() + 1); + derived(); + + const refs: WeakRef[] = []; + for (let i = 0; i < 5; i++) { + refs.push(setupAndReturnRef(source)); + } + + (globalThis as any).gc(); + await timeout(); + (globalThis as any).gc(); + + for (let i = 0; i < refs.length; i++) { + expect(refs[i].deref()).withContext(`cycle ${i}`).toBeUndefined(); + } + }); +}); From 2257e645087bec4f2bb418f6869af38ed4faf805 Mon Sep 17 00:00:00 2001 From: Milo Date: Fri, 17 Apr 2026 23:44:25 +0000 Subject: [PATCH 3/3] fixup! update leak test to use createWatch directly --- .../test/signals/signal_graph_leak_spec.ts | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/packages/core/test/signals/signal_graph_leak_spec.ts b/packages/core/test/signals/signal_graph_leak_spec.ts index ac22a3f8f202..0584f5453c30 100644 --- a/packages/core/test/signals/signal_graph_leak_spec.ts +++ b/packages/core/test/signals/signal_graph_leak_spec.ts @@ -7,40 +7,31 @@ */ import {computed, signal, WritableSignal} from '../../src/core'; -import {ReactiveNode, SIGNAL} from '../../primitives/signals'; +import {createWatch, SIGNAL} from '../../primitives/signals'; import {isBrowser, timeout} from '@angular/private/testing'; -import {flushEffects, resetEffects, testingEffect} from './effect_util'; - describe('signal graph: destroyed consumers should be GC-eligible', () => { if (isBrowser) { it('should pass', () => {}); return; } - afterEach(() => { - resetEffects(); - }); - function setupAndReturnRef(source: WritableSignal): WeakRef { - const destroyEffect = testingEffect(() => source()); - flushEffects(); + const watch = createWatch( + () => source(), + () => {}, + true, + ); + watch.run(); - const sourceNode = source[SIGNAL] as ReactiveNode; - let watchNode; - let link = (sourceNode as any).consumers; - while (link !== undefined) { - watchNode = link.consumer; - link = link.nextConsumer; - } - const ref = new WeakRef(watchNode!); + const ref = new WeakRef(watch[SIGNAL]); // Non-live computed that also reads source. const derived = computed(() => source() + 1); derived(); // Clearing the graph edges. - destroyEffect(); + watch.destroy(); return ref; }