From 035e9b2f34f9c160ac708676e242fd594056a509 Mon Sep 17 00:00:00 2001 From: Gabriel Valfridsson Date: Tue, 20 Oct 2026 16:19:02 +0000 Subject: [PATCH] fix(signals): restore deferred releases after cleanup throws --- .changeset/quiet-lazy-cleanups.md | 5 ++ packages/signals/src/core/core.ts | 7 ++- .../deferred-release-cleanup-throw.test.ts | 63 +++++++++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 .changeset/quiet-lazy-cleanups.md create mode 100644 packages/signals/tests/deferred-release-cleanup-throw.test.ts diff --git a/.changeset/quiet-lazy-cleanups.md b/.changeset/quiet-lazy-cleanups.md new file mode 100644 index 000000000..88aeb7840 --- /dev/null +++ b/.changeset/quiet-lazy-cleanups.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +Restore deferred lazy releases when an unfinished frame's cleanup throws, so later unrelated lazy memos still clean up when their last observer is removed. diff --git a/packages/signals/src/core/core.ts b/packages/signals/src/core/core.ts index 18a53fef4..03f14f0cb 100644 --- a/packages/signals/src/core/core.ts +++ b/packages/signals/src/core/core.ts @@ -400,8 +400,11 @@ export function recompute(el: Computed, create: boolean = false): void { // ends, if the pass doesn't read it again (#4142). const prev = deferredReleases; deferReleases((released = [])); - disposeChildren(el); - deferReleases(prev); + try { + disposeChildren(el); + } finally { + 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 new file mode 100644 index 000000000..3454f665f --- /dev/null +++ b/packages/signals/tests/deferred-release-cleanup-throw.test.ts @@ -0,0 +1,63 @@ +import { expect, it } from "vitest"; +import { + createMemo, + createRenderEffect, + createRoot, + createSignal, + flush, + onCleanup +} from "../src/index.js"; + +// #4164 defers last-reader releases while an unfinished pass's frame is +// dismantled. A cleanup throw must not leave that collector installed for +// later, unrelated disposal. +it.each([false, true])( + "a cleanup throw does not suppress unrelated lazy cleanup (throws: %s)", + throws => { + const failure = new Error("cleanup failed"); + const [version, setVersion] = createSignal(0); + const disposePending = createRoot(dispose => { + createMemo(() => { + version(); + onCleanup(() => { + if (throws) throw failure; + }); + return new Promise(() => {}); + }); + return dispose; + }); + let disposeOther: (() => void) | undefined; + try { + flush(); + setVersion(1); + if (throws) expect(() => flush()).toThrow(failure); + else expect(() => flush()).not.toThrow(); + + let cleaned = 0; + let stop!: () => void; + disposeOther = createRoot(dispose => { + const value = createMemo( + () => { + onCleanup(() => cleaned++); + return 1; + }, + { lazy: true } + ); + stop = createRoot(dispose => { + createRenderEffect(value, () => {}); + return dispose; + }); + return dispose; + }); + flush(); + stop(); + flush(); + // Its owner is still alive: losing the last observer releases it. + expect(cleaned).toBe(1); + } finally { + disposeOther?.(); + disposePending(); + flush(); + } + } +);