Skip to content

fix(signals): a released projection leaf leaves its firewall's companion set (#3503) - #3506

Merged
ryansolid merged 1 commit into
solidjs:nextfrom
brenelz:fix/companion-leaf-release-3503
Sep 17, 2026
Merged

ryansolid merged 1 commit into
solidjs:nextfrom
brenelz:fix/companion-leaf-release-3503

Conversation

@brenelz

@brenelz brenelz commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Fixes #3503.

markFirewallChildCompanions registers a leaf on its projection's permanent _companionChildren set the first time it is read through latest() or isPending(). When the last reader is disposed, the unobserved sweep removes the leaf from the store's node cache and unlinkFirewallChild takes it off the firewall child chain (#3351), but nothing removed it from the companion set. Later writes address a fresh leaf, so the obsolete leaf, its last value, and its companion signal/computed stayed reachable from the live projection.

The fix removes the leaf from the set inside unlinkFirewallChild. The set exists only so the snap can iterate the children someone asked verdicts of; a leaf the store no longer addresses can never be asked again, so dropping it changes no verdict. Under __TEST__ the INV-4 companionOwners registry is released at the same point so the in-suite GC test can observe the runtime behaviour (the issue notes this registry would otherwise mask it).

Reproduced the issue's script (ordinary / latest / isPending / nested, delete and overwrite variants) against the prod build on next 191a572: ordinary passes, the rest fail; all pass with this change.

  • Pin: packages/signals/tests/gc.test.ts (two cases, run with NODE_OPTIONS=--expose-gc npx vitest run tests/gc.test.ts; the existing should gc effect lazily case is broken on next independently and pnpm test:gc no longer finds the file under vitest 4).
  • Size: signals: + createStore cap 16.45 -> 16.50 KB (measured 16,477 B; next measures 16,426). Core floor and + isPending/latest unchanged.
  • Full signals suite: 197 files, 2736 passed.

🤖 Generated with Claude Code

@changeset-bot

changeset-bot Bot commented Sep 17, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6e9243c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@solidjs/signals Patch
test-integration Patch
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/compiler Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
solid-js Patch
@solidjs/universal Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codspeed

codspeed Bot commented Sep 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 175 untouched benchmarks


Comparing brenelz:fix/companion-leaf-release-3503 (6e9243c) with next (edafccb)

Open in CodSpeed

…ion set (solidjs#3503)

A store leaf read only through latest()/isPending() joins its projection's
`_companionChildren` set so the post-recompute snap can find it. The
unobserved sweep drops the leaf from the store cache and the firewall child
chain, but the set kept it — and with it the leaf's last value and its
companion nodes — for the projection's lifetime. `unlinkFirewallChild` now
removes the leaf from the set too (and from the test-only INV-4 registry,
which otherwise pins it under __TEST__).

Pin: gc.test.ts `releases an obsolete leaf value after a latest()/isPending()
reader is disposed (solidjs#3503)` — fails on next 191a572, passes with the fix.

Size: `+ createStore` 16.45 -> 16.50 KB, measured 16,477 B vs next's 16,426
(+51 brotli for one `?.delete(n)`; next had 24 B of headroom). Other scenarios
unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid
ryansolid force-pushed the fix/companion-leaf-release-3503 branch from 86f4bc4 to 6e9243c Compare September 17, 2026 04:51
@ryansolid
ryansolid merged commit d04da26 into solidjs:next Sep 17, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants