fix(signals): a lazy memo read only through a rebuilt child memo is not released mid-pass (#4142)

.changeset/lazy-memo-rebuilt-reader.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/signals": patch
+---
+
+fix(signals): a lazy memo read only through a rebuilt child memo is not released mid-pass (#4142)

packages/signals/src/core/async.ts

@@ -130,7 +130,7 @@ export function forEachDependent(
 // forever with zero subscribers (#2934). The node's own promise/iterator
 // callbacks handle their own release (settleAutodispose in handleAsync); this
 // covers derivatively-pending dependents, which have no callbacks of their own.
-function releaseIfSettledUnobserved(node: Computed<any>): void {
+export function releaseIfSettledUnobserved(node: Computed<any>): void {
   (node as any)._fn &&
     node._config & CONFIG_AUTO_DISPOSE &&
     !node._subs &&

packages/signals/src/core/core.ts

@@ -4,6 +4,7 @@ import {
   notifyStatus,
   parkLoadingWindow,
   releaseFlightTeardown,
+  releaseIfSettledUnobserved,
   settleErroredDependents,
   settlePendingSource,
   stageError
@@ -67,7 +68,14 @@ import {
   type Refreshable
 } from "./constants.js";
 import { NotReadyError } from "./error.js";
-import { clearDeps, dormantNodes, link, trimStaleDeps } from "./graph.js";
+import {
+  clearDeps,
+  deferReleases,
+  deferredReleases,
+  dormantNodes,
+  link,
+  trimStaleDeps
+} from "./graph.js";
 import {
   deleteFromHeap,
   enqueueSub,
@@ -289,6 +297,7 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
   // §12d: any recompute can clean a marked subscriber — invalidate skips.
   bumpNotifyEpoch();
   const isEffect = (el as any)._type;
+  let released: Computed<any>[] | undefined;
   // Lanes (plan sec. 28): the seat of a pass is its node's. A node carrying a lane's
   // derived value runs as the lane's whoever dirtied it — a sync write, a
   // boundary reset, a frame rerun — so its children and its result are the
@@ -385,8 +394,14 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
     // frame parked before it stays parked. Otherwise they ARE the frame:
     // park them until this pass commits. Tracked effects run after the seam
     // and own nothing to park.
-    if (isEffect === EFFECT_TRACKED || el._config & CONFIG_STAGED) disposeChildren(el);
-    else if (el._firstChild !== null || el._disposal !== null) parkChildren(el);
+    if (isEffect === EFFECT_TRACKED || el._config & CONFIG_STAGED) {
+      // A lazy node only those children read is released when this pass
+      // ends, if the pass doesn't read it again (#4142).
+      const prev = deferredReleases;
+      deferReleases((released = []));
+      disposeChildren(el);
+      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).
     if (el._config & CONFIG_STAGED_ERROR) {
@@ -546,6 +561,7 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
           REACTIVE_PROBE_UNANSWERED)) |
       (create ? el._flags & REACTIVE_SNAPSHOT_STALE : 0);
     context = oldcontext;
+    if (released) for (const node of released) releaseIfSettledUnobserved(node);
     // A19 exc. 2: a pass that went pending on a `refresh()` re-asks the
     // question already answered — quiet for the verdict; any other pending
     // pass is a new question. Settled, there is nothing to classify.

packages/signals/src/core/graph.ts

@@ -43,12 +43,14 @@ export function unlinkSubs(link: Link): Link | null {
       // read. The settle path runs this same last-one-out check when that
       // observer releases (the untracked-read dormancy sweep guards on
       // pending identically).
+      // During a pass's teardown of its unfinished frame, the release waits
+      // for the pass to end (`deferredReleases`, #4142).
       const c = dep as Computed<any>;
       (c as any)._fn &&
         c._config & CONFIG_AUTO_DISPOSE &&
         !(c._flags & REACTIVE_ZOMBIE) &&
         !(c._statusFlags & STATUS_PENDING) &&
-        unobserved(c);
+        (deferredReleases ? deferredReleases.push(c) : unobserved(c));
     }
   }
   return nextDep;
@@ -80,6 +82,18 @@ export function clearDeps(el: Computed<unknown>): void {
   el._depsTail = null;
 }
 
+/**
+ * Last-one-out releases held back while a pass tears down the frame of an
+ * earlier pass that never committed (#4142). The pass is about to rebuild
+ * the same children, which read the same lazy nodes again. Released on the
+ * spot, a lazy async memo whose only reader was the old frame's child memo
+ * was torn down, re-ran on the new child's read, went pending, and the cycle
+ * repeated forever. `recompute` collects them around that teardown and
+ * releases what is still unobserved when the pass ends.
+ */
+export let deferredReleases: Computed<any>[] | null = null;
+export const deferReleases = (list: Computed<any>[] | null) => (deferredReleases = list);
+
 export function unobserved(el: Computed<unknown>) {
   deleteFromHeap(el, dirtyQueue);
   clearDeps(el);

packages/signals/tests/dormant-memo-chain-3554.test.ts

@@ -219,7 +219,11 @@ it("a lazy memo made dormant and reawakened while its owner's held children drai
   setTick(1);
   flush();
   expect(cleaned).toEqual(["sibling1"]);
-  expect(chain(owner)).toEqual(["reader2", "lazy2", "sibling2", "lazy1"]);
+  // lazy1's last reader dies in the same teardown, so its release waits for
+  // the pass to end (#4142). It is never dormant mid-drain, the cleanup's
+  // read finds it alive, and it dies with the frame it belongs to instead of
+  // reawakening into the new one.
+  expect(chain(owner)).toEqual(["reader2", "lazy2", "sibling2"]);
 
   dispose();
   expect(cleaned).toEqual(["sibling1", "sibling2"]);

packages/signals/tests/lazy-memo-rebuilt-reader-4142.test.ts

@@ -0,0 +1,76 @@
+// #4142: a lazy async memo whose only reader is a child memo that an
+// unfinished pass rebuilds. The pass tears the old child down before it
+// creates the new one, so the lazy memo briefly had no subscriber and was
+// released on the spot; the new child's read then re-ran it, it went pending
+// again, and the cycle repeated forever.
+import { expect, it } from "vitest";
+import {
+  createMemo,
+  createRenderEffect,
+  createRoot,
+  createSignal,
+  flush,
+  onCleanup
+} from "../src/index.js";
+
+const wait = (ms: number) => new Promise(r => setTimeout(r, ms));
+
+it("a lazy async memo read only through a rebuilt child memo runs once", async () => {
+  let runs = 0;
+  const seen: string[] = [];
+  const dispose = createRoot(d => {
+    const user = createMemo(
+      async () => {
+        runs++;
+        await wait(5);
+        return { name: "admin" };
+      },
+      { lazy: true }
+    );
+    createRenderEffect(
+      () => {
+        const cond = createMemo(() => !!user());
+        return cond() ? "yes" : "no";
+      },
+      v => {
+        seen.push(v);
+      }
+    );
+    return d;
+  });
+  flush();
+  for (let i = 0; i < 10; i++) await wait(5);
+  expect(runs).toBe(1);
+  expect(seen).toEqual(["yes"]);
+  dispose();
+});
+
+it("a lazy memo the rebuilt pass stops reading is released when the pass ends", () => {
+  let released = 0;
+  let setTick!: (v: number) => void;
+  const dispose = createRoot(d => {
+    const [tick, _setTick] = createSignal(0);
+    setTick = _setTick;
+    const lazy = createMemo(
+      () => {
+        onCleanup(() => released++);
+        return 1;
+      },
+      { lazy: true }
+    );
+    // A pass that never settles: each new pass tears down the last one's
+    // children on the spot.
+    const held = createMemo(() => {
+      if (tick() === 0) createMemo(() => lazy())();
+      return new Promise<number>(() => {});
+    });
+    createRenderEffect(held, () => {});
+    return d;
+  });
+  flush();
+  expect(released).toBe(0);
+  setTick(1);
+  flush();
+  expect(released).toBe(1);
+  dispose();
+});

packages/web/test/lazy-memo-loading-loop-4142.spec.tsx

@@ -0,0 +1,45 @@
+/**
+ * @jsxImportSource @solidjs/web
+ * @vitest-environment jsdom
+ */
+// #4142, the report's playground: a lazy async memo read in both operands of
+// `user() && <User data={user()} />` under <Show>/<Loading>. The compiled
+// condition is a child memo of the hole's pass, and rebuilding it released
+// the lazy memo, which re-fetched forever behind the fallback.
+import { expect, test } from "vitest";
+import { createMemo, createSignal, flush, Loading, Show } from "solid-js";
+import { render } from "../src/index.js";
+
+const wait = (ms: number) => new Promise(r => setTimeout(r, ms));
+
+test("a lazy async memo read on both sides of && fetches once and shows", async () => {
+  let fetches = 0;
+  const fetchUser = async () => {
+    fetches++;
+    await wait(5);
+    return { name: "admin" };
+  };
+  const User = (props: { data: { name: string } }) => <div>Name: {props.data.name}</div>;
+  const [showUser, setShowUser] = createSignal(false);
+  const user = createMemo(fetchUser, { lazy: true });
+  const div = document.createElement("div");
+  document.body.appendChild(div);
+  const dispose = render(
+    () => (
+      <div>
+        <input type="checkbox" onChange={() => setShowUser(x => !x)} />
+        <Show when={showUser()}>
+          <Loading fallback="loading">{user() && <User data={user()} />}</Loading>
+        </Show>
+      </div>
+    ),
+    div
+  );
+  div.querySelector("input")!.dispatchEvent(new Event("change", { bubbles: true }));
+  flush();
+  for (let i = 0; i < 10; i++) await wait(5);
+  expect(fetches).toBe(1);
+  expect(div.textContent).toContain("Name: admin");
+  dispose();
+  div.remove();
+});

scripts/size/floor-caps.json

@@ -1,23 +1,23 @@
 {
   "signals: core floor (createSignal/Memo/Effect/Root/flush)": {
-    "cap": "7.27 KB",
-    "minified": 19827
+    "cap": "7.35 KB",
+    "minified": 19934
   },
   "app: render + one signal (the simple-app floor)": {
-    "cap": "9.59 KB",
-    "minified": 26715
+    "cap": "9.66 KB",
+    "minified": 26824
   },
   "app: hydrating (no stores) with Show/For/Loading/Errored/lazy": {
-    "cap": "18.24 KB",
-    "minified": 53291
+    "cap": "18.29 KB",
+    "minified": 53401
   },
   "page: base server components (hydrating + dynamic + frames + sf reference)": {
-    "cap": "33.12 KB",
-    "minified": 101246
+    "cap": "33.16 KB",
+    "minified": 101352
   },
   "page: live server components (base + live/GET + action + isPending/latest)": {
-    "cap": "37.37 KB",
-    "minified": 114782
+    "cap": "37.46 KB",
+    "minified": 114901
   },
   "server: floor (getRequestEvent + isServer)": {
     "cap": "1.34 KB",