From 9a6ca296ecba24c058f0395734c533d467b60f82 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 19:38:45 +0000 Subject: [PATCH 1/3] fix(signals): release lazy nodes collected before a throwing cleanup #4176 restores the release collector when a cleanup throws while an unfinished pass's frame is dismantled, but the throw also skips the drain at the end of the pass, so lazy nodes the frame's children released before the throw were never released. Drain them before rethrowing. Also adds the solid-js changeset for #4183's docs, which ship in the package. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018LY7F9czoRqdMEvk2QwPr1 --- .changeset/deferred-release-drain.md | 5 +++ .changeset/docs-from-deferred-settled.md | 5 +++ packages/signals/src/core/core.ts | 7 +++- .../deferred-release-cleanup-throw.test.ts | 36 +++++++++++++++++++ 4 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 .changeset/deferred-release-drain.md create mode 100644 .changeset/docs-from-deferred-settled.md diff --git a/.changeset/deferred-release-drain.md b/.changeset/deferred-release-drain.md new file mode 100644 index 000000000..9aa201de3 --- /dev/null +++ b/.changeset/deferred-release-drain.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +A cleanup that throws while an unfinished pass's frame is dismantled no longer strands the lazy nodes released before it: they are released before the error propagates. diff --git a/.changeset/docs-from-deferred-settled.md b/.changeset/docs-from-deferred-settled.md new file mode 100644 index 000000000..611ac622f --- /dev/null +++ b/.changeset/docs-from-deferred-settled.md @@ -0,0 +1,5 @@ +--- +"solid-js": patch +--- + +The migration guide notes when a `from()` async generator's cleanup runs, the cheatsheet adds `createDeferred`, and the docs note that `SETTLED_CLEANUP_UNOWNED` throws in development (#4183). diff --git a/packages/signals/src/core/core.ts b/packages/signals/src/core/core.ts index 4cb92f61f..7baac6747 100644 --- a/packages/signals/src/core/core.ts +++ b/packages/signals/src/core/core.ts @@ -411,9 +411,14 @@ export function recompute(el: Computed, create: boolean = false): void { deferReleases((released = [])); try { disposeChildren(el); - } finally { + } catch (e) { + // The throw skips the drain at the end of the pass: release what the + // cleanups before it collected now. deferReleases(prev); + for (const node of released) releaseIfSettledUnobserved(node); + throw e; } + deferReleases(prev); } else if (el._firstChild !== null || el._disposal !== null) parkChildren(el); // An error a pass staged (D-41) is replaced by this pass's answer: a // value, a flight, or an error of its own (#4109, #4145). diff --git a/packages/signals/tests/deferred-release-cleanup-throw.test.ts b/packages/signals/tests/deferred-release-cleanup-throw.test.ts index 3454f665f..9708aa737 100644 --- a/packages/signals/tests/deferred-release-cleanup-throw.test.ts +++ b/packages/signals/tests/deferred-release-cleanup-throw.test.ts @@ -61,3 +61,39 @@ it.each([false, true])( } } ); + +// A lazy node the dismantled frame's children released before the throw is +// still released: the throw skips the drain at the end of the pass. +it("a cleanup throw still releases lazy nodes collected before it", () => { + const failure = new Error("cleanup failed"); + const [version, setVersion] = createSignal(0); + let cleaned = 0; + const dispose = createRoot(dispose => { + const lazy = createMemo( + () => { + onCleanup(() => cleaned++); + return 1; + }, + { lazy: true } + ); + createMemo(() => { + if (version() === 0) { + createMemo(() => lazy()); + onCleanup(() => { + throw failure; + }); + } + return new Promise(() => {}); + }); + return dispose; + }); + try { + flush(); + setVersion(1); + expect(() => flush()).toThrow(failure); + expect(cleaned).toBe(1); + } finally { + dispose(); + flush(); + } +}); From f2be3ec192e73c6340d0bfff718b261f09e931dd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 19:50:12 +0000 Subject: [PATCH 2/3] revert the lazy-release drain; keep only the #4183 changeset The drain costs about 35 B minified in every signals scenario, including the simple-app floor, for a leak that only follows a cleanup that throws. Held for a size decision. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018LY7F9czoRqdMEvk2QwPr1 --- .changeset/deferred-release-drain.md | 5 --- packages/signals/src/core/core.ts | 7 +--- .../deferred-release-cleanup-throw.test.ts | 36 ------------------- 3 files changed, 1 insertion(+), 47 deletions(-) delete mode 100644 .changeset/deferred-release-drain.md diff --git a/.changeset/deferred-release-drain.md b/.changeset/deferred-release-drain.md deleted file mode 100644 index 9aa201de3..000000000 --- a/.changeset/deferred-release-drain.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@solidjs/signals": patch ---- - -A cleanup that throws while an unfinished pass's frame is dismantled no longer strands the lazy nodes released before it: they are released before the error propagates. diff --git a/packages/signals/src/core/core.ts b/packages/signals/src/core/core.ts index 7baac6747..4cb92f61f 100644 --- a/packages/signals/src/core/core.ts +++ b/packages/signals/src/core/core.ts @@ -411,14 +411,9 @@ export function recompute(el: Computed, create: boolean = false): void { deferReleases((released = [])); try { disposeChildren(el); - } catch (e) { - // The throw skips the drain at the end of the pass: release what the - // cleanups before it collected now. + } finally { deferReleases(prev); - for (const node of released) releaseIfSettledUnobserved(node); - throw e; } - deferReleases(prev); } else if (el._firstChild !== null || el._disposal !== null) parkChildren(el); // An error a pass staged (D-41) is replaced by this pass's answer: a // value, a flight, or an error of its own (#4109, #4145). diff --git a/packages/signals/tests/deferred-release-cleanup-throw.test.ts b/packages/signals/tests/deferred-release-cleanup-throw.test.ts index 9708aa737..3454f665f 100644 --- a/packages/signals/tests/deferred-release-cleanup-throw.test.ts +++ b/packages/signals/tests/deferred-release-cleanup-throw.test.ts @@ -61,39 +61,3 @@ it.each([false, true])( } } ); - -// A lazy node the dismantled frame's children released before the throw is -// still released: the throw skips the drain at the end of the pass. -it("a cleanup throw still releases lazy nodes collected before it", () => { - const failure = new Error("cleanup failed"); - const [version, setVersion] = createSignal(0); - let cleaned = 0; - const dispose = createRoot(dispose => { - const lazy = createMemo( - () => { - onCleanup(() => cleaned++); - return 1; - }, - { lazy: true } - ); - createMemo(() => { - if (version() === 0) { - createMemo(() => lazy()); - onCleanup(() => { - throw failure; - }); - } - return new Promise(() => {}); - }); - return dispose; - }); - try { - flush(); - setVersion(1); - expect(() => flush()).toThrow(failure); - expect(cleaned).toBe(1); - } finally { - dispose(); - flush(); - } -}); From 08b39f40c723d56c336978c636eb88ca45e13901 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 20:09:59 +0000 Subject: [PATCH 3/3] chore(size): re-record the compiled CSR cap after #4188, #4207 and #4176 Each passed alone; together they put next 1 B past the 20 B minified allowance (81,159 B against 81,138 recorded). Accepted by the maintainer. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018LY7F9czoRqdMEvk2QwPr1 --- scripts/size/scenarios.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/size/scenarios.js b/scripts/size/scenarios.js index 8b917c4c3..3794728de 100644 --- a/scripts/size/scenarios.js +++ b/scripts/size/scenarios.js @@ -3974,8 +3974,9 @@ module.exports = [ // Every comparator call guarded, default comparators on a fast path (#4111, 2026-10-20, over next @ 6371b910): cap 26.10 KB unchanged, recorded minified 81,083 -> 81,112 B (+29 B), measured in CI at 26,086 B brotli. `isSame` is the one guarded comparator call (a NotReady read is a change, any other throw goes to the node's boundary, A3), with `isEqual` on a fast path so the propagation benches keep next's speed. Size-Exception accepted by the maintainer (2026-10-19). Cap at measured + 10 B rounded up to 0.01 KB, never lowered here; recorded minified = measured. // Writable memo write clears its error without a torn read (#4110, 2026-10-20, over next @ 7014aca4, accepted by the maintainer): 26.10 KB (unchanged), recorded minified 81,112 -> 81,172 B, measured locally (linux) at 26,087 B brotli / 81,172 B minified (+60 B minified over the recorded size), which includes #4163's +9 B merged without re-recording: a write to an errored writable memo recovers it at commit and keeps the error shown until then, and clears an error a pass staged under the same hold. Correctness; matches CI's numbers. // Verdict-mount lane provenance for nested first passes (#4166, 2026-10-20, over next @ 5a4b2e85): 26.10 KB -> 26.11 KB, recorded minified 81,172 -> 81,138 B (-34 B), measured in CI at 26,092 B brotli. A module-local laneRoot carries the mount's lane through nested first passes, gated on the lane engine, so bundles without it shrink. Size-Exception accepted by the maintainer (2026-10-20). Cap at measured + 10 B rounded up to 0.01 KB, never lowered here; recorded minified = measured. - limit: "26.11 KB", - capMinified: 81138, + // Re-recorded on next @ 0222d684 (2026-10-21, accepted by the maintainer): 26.11 KB -> 26.14 KB, recorded minified 81,138 -> 81,159 B (+21 B), measured in CI at 26,123 B brotli. #4188 (reaction arms captured before tracking reads), #4207 (#4203, unread pending memo under a verdict mount) and #4176 (deferred releases restored after a throwing cleanup) each passed alone and together put next 1 B past the 20 B minified allowance. Cap at measured + 10 B rounded up to 0.01 KB; recorded minified = measured. + limit: "26.14 KB", + capMinified: 81159, alias }, {