solidjs / solidjs/solid

Dormant lazy memo re-splices a stale _nextSibling into parent._firstChild on second dormancy, orphaning live siblings

Open
#3,554 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
36.1k
Forks
1.1k
Avg merge
9h 18m
Merged PRs (30d)
195

Description

What happens

An auto-dispose memo (createMemo(fn, { lazy: true }), i.e. CONFIG_AUTO_DISPOSE) that goes dormant — its last subscriber leaves, unlinkSubs calls unobserved(node)disposeChildren(node, true) — is spliced out of its owner's child chain (_prevSibling/_nextSibling/parent._firstChild). Its own _nextSibling is deliberately left intact so outer walks still advance, and _prevSibling is nulled.

When the memo is later read again, prepareComputed sees REACTIVE_DISPOSED | CONFIG_AUTO_DISPOSE and reawakens it via recompute(comp, true). That wipes REACTIVE_DISPOSED from _flags but does not relink the node into the owner's chain. The node is now "live" by flags but sits outside the chain with _prevSibling === null and a stale _nextSibling.

On the second dormancy, disposeChildren runs the splice again. With _prevSibling === null it takes the head branch and writes the stale _nextSibling into parent._firstChild. Every live sibling ahead of the memo in the chain (everything created after it) is orphaned: never disposed by the owner, never zombified by the owner's next rerun, still subscribed to its sources.

Observable consequence: a sibling effect keeps rerunning after the root's dispose().

This is independent of the zombie/REACTIVE_ZOMBIE bug in #3543 (fixed in #3547; the #3545 variant was rejected). It is the plain dormancy lifecycle, no transactions or zombies involved — the same head-clobbering write, reached through a different path. The owner-chain-head DEV invariant added in #3552 now catches it under __TEST__; in prod/observe tiers the write still goes through silently.

Repro (vitest, packages/signals/tests/)

Children are prepended, so creation order X, m1, reader, Y yields chain [Y, reader, m1, X]. reader toggles its read of m1 off / on / off.

import {
  DEV,
  createEffect,
  createMemo,
  createRenderEffect,
  createRoot,
  createSignal,
  flush,
  getOwner
} from "../src/index.js";

// Same walk as DEV.getChildren; done inline so it also works under
// SIGNALS_TIER=observe (no DEV surface, invariants compiled out).
function chain(root: any): string[] {
  const out: string[] = [];
  for (let c = root._firstChild; c; c = c._nextSibling) out.push(c._name ?? "?");
  return out;
}

it("dormant lazy memo: second dormancy orphans live siblings", () => {
  const runs = { X: 0, Y: 0, m1: 0 };
  let rootOwner!: any;
  let setRead!: (v: boolean) => void;
  let setTick!: (v: number) => void;

  const dispose = createRoot(d => {
    rootOwner = getOwner()!;
    const [read, _setRead] = createSignal(true);
    const [tick, _setTick] = createSignal(0);
    setRead = _setRead;
    setTick = _setTick;

    createEffect(() => tick(), () => { runs.X++; });
    (rootOwner._firstChild as any)._name = "X";

    const m1 = createMemo(() => { runs.m1++; return tick() * 2; }, { lazy: true });
    (rootOwner._firstChild as any)._name = "m1";

    createRenderEffect(() => (read() ? m1() : -1), () => {});
    (rootOwner._firstChild as any)._name = "reader";

    createEffect(() => tick(), () => { runs.Y++; });
    (rootOwner._firstChild as any)._name = "Y";
    return d;
  });
  flush();
  console.log("initial chain:", JSON.stringify(chain(rootOwner)));

  setRead(false); flush(); // 1st dormancy: m1 spliced out
  console.log("after 1st dormancy:", JSON.stringify(chain(rootOwner)));

  setRead(true); flush();  // reawaken: recompute(m1, true), NOT relinked
  console.log("after reawaken:", JSON.stringify(chain(rootOwner)));

  let thrown: unknown = null;
  try { setRead(false); flush(); } catch (e) { thrown = e; } // 2nd dormancy
  console.log("after 2nd dormancy:", JSON.stringify(chain(rootOwner)));
  console.log("thrown:", thrown ? String(thrown) : "(nothing)");

  const y0 = runs.Y, x0 = runs.X;
  dispose();
  setTick(1); flush();
  setTick(2); flush();
  console.log("post-dispose runs — X:", runs.X - x0, "Y:", runs.Y - y0, "(expected 0 / 0)");

  expect(thrown).toBeNull();
  expect(chain(rootOwner)).toEqual(["Y", "reader", "X"]);
  expect(runs.Y - y0).toBe(0);
});

Exact output on next (be46a04de, includes #3552)

Default dev tier (pnpm --filter @solidjs/signals exec vitest run tests/<file>; __TEST__ throws on invariant violation):

initial chain: ["Y","reader","m1","X"]
after 1st dormancy: ["Y","reader","X"]
after reawaken: ["Y","reader","X"]
after 2nd dormancy: ["Y","reader","X"]
thrown: Error: [INVARIANT_VIOLATION] owner-chain-head: head node is not parent._firstChild — a node was spliced while flagged live but not in the chain (see #3543)
post-dispose runs — X: 0 Y: 0 (expected 0 / 0)

(The throw aborts the splice before the bad write, so the chain survives here — but the flush is torn by an exception.)

SIGNALS_TIER=observe (__DEV__ false → invariant compiled out; this is what prod/observe bundles do):

initial chain: ["Y","reader","m1","X"]
after 1st dormancy: ["Y","reader","X"]
after reawaken: ["Y","reader","X"]
after 2nd dormancy: ["X"]
thrown: (nothing)
post-dispose runs — X: 0 Y: 2 (expected 0 / 0)

Y and reader are orphaned; Y keeps rerunning after dispose(). This matches the symptom observed by the #3552 author while reviewing #3543/#3545 (pre-#3552 next behaves the same way, minus the invariant).

Code locations (on next @ be46a04de)

packages/signals/src/core/owner.ts

  • disposeChildren — L71. The self-splice block is L134–157:
    • L141–142: const prev = node._prevSibling; const next = node._nextSibling;
    • L147–152: owner-chain-head invariant from #3552 (prev !== null || node._parent._firstChild === node)
    • L153–154: if (prev !== null) prev._nextSibling = next; else node._parent._firstChild = next; ← the clobbering write on the second dormancy
    • L156: node._prevSibling = null; — after this, the reawakened node always takes the head branch next time
  • L130–133 comment documents the intent: _nextSibling is left intact for outer walks. That is exactly the stale pointer that gets written to the head later.

packages/signals/src/core/graph.ts

  • L39–43: unlinkSubs last-one-out — c._config & CONFIG_AUTO_DISPOSE && !(c._flags & REACTIVE_ZOMBIE) && !(c._statusFlags & STATUS_PENDING) && unobserved(c) — the dormancy trigger when the last subscriber leaves.
  • L75–79: unobserved(el)deleteFromHeap, clearDeps, L78 disposeChildren(el, true) — the self-dispose that runs the splice.
  • L103–121: sweepDormant — the untracked-read dormancy path (queued from core.ts L2089), reaches the same unobserved.

packages/signals/src/core/core.ts

  • L915 / L953: options?.lazyCONFIG_AUTO_DISPOSE on the node's _config.
  • L1186–1195: the only place a computed is linked into context._firstChild — creation time. Nothing relinks later.
  • L1507–1522 prepareComputed; L1518: if (comp._config & CONFIG_AUTO_DISPOSE) recompute(comp as Computed<any>, true); ← the reawaken; no chain relink.
  • L499: recompute wipes _flags (el._flags = (el._flags & REACTIVE_ZOMBIE) | (create ? el._flags & REACTIVE_SNAPSHOT_STALE : 0)) — this is where REACTIVE_DISPOSED is cleared, making the node "live" again for the next disposeChildren splice guard.

Candidate fixes (no choice made)

  1. Relink on reawaken — in the prepareComputed dormant branch (or inside recompute(_, true) for a REACTIVE_DISPOSED node), push the node back onto _parent._firstChild (and clear the stale _nextSibling) before recomputing, so the chain is a true invariant of "live" nodes. Tradeoff: correct owner-disposal semantics for reawakened memos (the owner's dispose() reaches them again and can strip AUTO_DISPOSE), but it moves the node to the head of the chain (creation order is no longer preserved) and adds work to the reawaken path.
  2. Skip the splice for dormant nodes — in disposeChildren, gate the self-splice on !(node._config & CONFIG_AUTO_DISPOSE) (dormancy is not death; leave the node in the chain and let the owner's teardown / the REACTIVE_DISPOSED early-return handle it). Tradeoff: smaller change and preserves chain order, but dormant nodes are retained by their owner until the owner is disposed (no O(1) reclaim), and getChildren/devtools will list dormant nodes as children.

Either way, the owner-chain-head invariant from #3552 should be kept as the regression pin, and a test in the shape above should go green under both dev and SIGNALS_TIER=observe.

Refs #3543 #3547 #3552

Filed by Claude via Cursor from the #3552 investigation

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with disposeChildren in packages/signals/src/core/owner.ts, then trace unlinkSubs and unobserved in graph.ts and the dormant branch of prepareComputed in core.ts. Run the supplied regression shape under the dev and SIGNALS_TIER=observe configurations; done means no owner-chain corruption, no orphaned sibling effects after dispose(), and the invariant remains covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend, performance, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.