Skip to content

fix(core): prevent dangling prevConsumer reference from leaking v2#68681

Open
milomg wants to merge 3 commits into
angular:mainfrom
milomg:fix/signal-prevConsumer-leak
Open

fix(core): prevent dangling prevConsumer reference from leaking v2#68681
milomg wants to merge 3 commits into
angular:mainfrom
milomg:fix/signal-prevConsumer-leak

Conversation

@milomg
Copy link
Copy Markdown
Member

@milomg milomg commented May 12, 2026

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

The current signals implementation leaks

What is the new behavior?

See the analysis in #68137, this is just a new PR so that I can add additional edits to the test implementation

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

stewartmcgown and others added 3 commits April 11, 2026 00:36
…oyed views

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
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
@pullapprove pullapprove Bot requested review from atscott, crisbeto and csmick May 12, 2026 00:13
@angular-robot angular-robot Bot added area: core Issues related to the framework runtime requires: TGP This PR requires a passing TGP before merging is allowed labels May 12, 2026
@ngbot ngbot Bot added this to the Backlog milestone May 12, 2026
@milomg milomg changed the title Fix/signal prev consumer leak v2 Fix signal prev consumer leak v2 May 12, 2026
@milomg milomg changed the title Fix signal prev consumer leak v2 fix(core): prevent dangling prevConsumer reference from leaking v2 May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: core Issues related to the framework runtime requires: TGP This PR requires a passing TGP before merging is allowed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants