Skip to content

Commit ca16891

Browse files
Refuse re-entering a finished transaction; attribute the flush loop guard (#3140)
Companion to #3143 (stamps cleared when pending values commit): a dead _transition reference can still reach initTransition from outside the commit path — merged _done forwarding chains, async settles racing completion — and setSignal re-opens a node's stamped transaction before the value-equal bail, so re-activating the corpse spun the drain loop (dev threw the loop guard, production hung the tab). initTransition now refuses a transaction whose chased _done chain ends in true, as a bare return: redirecting to a fresh batch would re-arm the loop with a new identity each pass (measured by the reporter). The dev loop guard now reports what kept the loop alive — scheduled work vs an active transition, done-state, queue counts, last staged node. The corpse signature reads 'done=true, pending=0'. Dev-only, tree-shaken from prod. White-box pins for both layers; one size budget ratcheted 26.1 -> 26.15 KB (~25 B across this and #3141). Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 2023daa commit ca16891

5 files changed

Lines changed: 158 additions & 5 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/signals": patch
3+
---
4+
5+
Harden the transaction-stamp lifecycle around #3140 (companion to #3143, which clears `_transition` stamps when pending values commit). `initTransition` now refuses a transaction whose `_done` chain ends in `true` — the belt for dead references that survive outside the commit path (merged forwarding chains, async settles racing completion), since `setSignal` re-opens a node's stamped transaction before the value-equal bail and re-activating a corpse spins the flush drain loop (dev threw the loop guard; production hung). The dev loop guard also now reports what kept the loop alive — transition done-state, queue counts, and the last staged node — instead of only that it happened.

packages/signals/INTERNALS-ASYNC-STATE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ A `Signal`/`Computed` participating in async/transitions carries:
2828
| `_latestValueComputed` | Lazily-created companion: shadow computed for `latest()` | `getLatestValueComputed`, written via `syncCompanions` |
2929
| `_parentSource` | Companion → owner backlink (also store leaf → firewall chains) | companion creation |
3030
| `_optimisticLane` | Lane this node currently belongs to | `assignOrMergeLane`, cleared by `resolveLane` (stale), `resolveOptimisticNodes`, `cleanupCompletedLanes` |
31-
| `_transition` | Transition holding this node's pending state | `initTransition`, `reassignPendingTransition`, cleared by `resolveOptimisticNodes` |
31+
| `_transition` | Transition holding this node's pending state | `initTransition`, `reassignPendingTransition`, cleared by `resolveOptimisticNodes` and the `commitPendingNodes` loop (#3140/#3143: a stamp never outlives its transaction — a committed value needs no affiliation, and a dangling stamp let any later write, even a value-equal no-op, resurrect the finished transaction; `initTransition` refuses `_done === true` as the belt) |
3232

3333
Semantics of the `(_pendingValue, _overrideValue)` pair for an optimistic node
3434
(see §5e — revert targets were eliminated 2026-07-07):

packages/signals/src/core/scheduler.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -672,8 +672,17 @@ export class GlobalQueue extends Queue {
672672
return false;
673673
}
674674
initTransition(transition?: Transition | null): void {
675-
if (transition) transition = currentTransition(transition);
676-
if (transition && transition === activeTransition) return;
675+
if (transition) {
676+
transition = currentTransition(transition);
677+
// A finished transaction cannot be re-entered: its state is committed
678+
// or reverted, so "rejoining" it (A26) is meaningless and re-activating
679+
// it spins the drain loop (#3140). The refusal must be a bare return —
680+
// redirecting the caller to a fresh batch would re-arm the loop with a
681+
// new transaction identity each pass. Stamps are cleared at commit, so
682+
// this is a belt for paths that hand over a chased-dead reference
683+
// (merged chains, async settles racing completion).
684+
if (transition._done === true || transition === activeTransition) return;
685+
}
677686
if (!transition && activeTransition && activeTransition._time === clock) return;
678687
if (!activeTransition) {
679688
activeTransition = transition ?? createBatch();
@@ -730,9 +739,15 @@ export class GlobalQueue extends Queue {
730739
}
731740

732741
export function queuePendingNode(node: Signal<any>): void {
742+
if (__DEV__) lastStagedNodeName = (node as any)._name ?? null;
733743
currentBatch._pendingNodes.push(node);
734744
}
735745

746+
// Dev-only attribution for the flush loop guard (#3140): when the guard
747+
// trips, naming what the loop kept chewing on lets the app author attribute
748+
// the runaway without patching dist.
749+
let lastStagedNodeName: string | null = null;
750+
736751
// Sticky: flips true on the first refresh() ever (the only setter of
737752
// REACTIVE_REASK) so the hot notification loop skips the per-subscriber flag
738753
// clear entirely in apps that never refresh.
@@ -854,6 +869,12 @@ function commitPendingNodes() {
854869
for (let i = 0; i < pendingNodes.length; i++) {
855870
const node = pendingNodes[i];
856871
commitPendingNode(node);
872+
// The stamp dies with the commit (#3143) — symmetric with
873+
// resolveOptimisticNodes clearing optimistic stamps. A stamp outliving
874+
// its transaction let any later write (even a value-equal no-op, which
875+
// re-opens before the equality bail) resurrect the finished transaction;
876+
// a boundary flag rewritten every finalize pass then spun the drain loop
877+
// forever (#3140).
857878
node._transition = null;
858879
}
859880
pendingNodes.length = 0;
@@ -1029,7 +1050,22 @@ export function flush<T>(fn?: () => T): T | void {
10291050
// `flush()` is an explicit drain point, so it must also process an active
10301051
// transition even if no microtask was scheduled for it yet.
10311052
while (scheduled || activeTransition) {
1032-
if (__DEV__ && ++count === 1e5) throw new Error("Potential Infinite Loop Detected.");
1053+
if (__DEV__ && ++count === 1e5) {
1054+
// Attribution beats a bare guard (#3140): say what kept the loop alive.
1055+
// A completed transition being re-activated reads `done=true` here —
1056+
// the corpse-revival signature — while application-driven runaways
1057+
// (#2843) usually show staged work naming the culprit node.
1058+
const t = activeTransition as any;
1059+
throw new Error(
1060+
`Potential Infinite Loop Detected. Kept alive by ${
1061+
scheduled ? "scheduled work" : "an active transition"
1062+
}${
1063+
t
1064+
? `; transition: done=${t._done === true}, pending=${t._pendingNodes.length}, optimistic=${t._optimisticNodes.length}, asyncReporters=${t._asyncReporters.size}`
1065+
: ""
1066+
}${lastStagedNodeName ? `; last staged node: ${lastStagedNodeName}` : ""}`
1067+
);
1068+
}
10331069
globalQueue.flush();
10341070
}
10351071
}
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/**
2+
* #3140: a `_transition` stamp must not outlive its transaction.
3+
*
4+
* `commitPendingNode` committed the value but left `_transition` pointing at
5+
* the finished transaction (optimistic stamps were cleared at completion;
6+
* pending stamps were not). `setSignal` re-opens a node's stamped transaction
7+
* BEFORE the value-equal bail, so a later no-op write resurrected the corpse:
8+
* the drain loop saw a live transition again, completed it, and the next
9+
* finalize pass (a Loading boundary's `_checkSources` rewrites its flag with
10+
* the same value on every pass) revived it again — the dev build threw the
11+
* flush loop guard, production hung the tab.
12+
*
13+
* The wild stamping race did not reduce to a minimal reproduction (see the
14+
* issue), so each layer is pinned directly:
15+
* - commit clears the stamp (symmetry with resolveOptimisticNodes);
16+
* - a write finding a dead stamp — however it survived — must not
17+
* re-activate the finished transaction.
18+
*/
19+
import { describe, expect, it } from "vitest";
20+
import { action, createSignal, flush } from "../src/index.js";
21+
import { createRoot } from "../src/index.js";
22+
import { setSignal, signal, type Signal } from "../src/core/index.js";
23+
import { activeTransition, type Transition } from "../src/core/scheduler.js";
24+
25+
function deferred<T = void>() {
26+
let resolve!: (v: T) => void;
27+
const promise = new Promise<T>(r => (resolve = r));
28+
return { promise, resolve };
29+
}
30+
31+
/** Runs an action to completion and returns its (now finished) transaction. */
32+
async function completedTransition(): Promise<Transition> {
33+
const gate = deferred();
34+
let captured: Transition | null = null;
35+
let start!: () => Promise<unknown>;
36+
createRoot(() => {
37+
start = action(function* () {
38+
captured = activeTransition;
39+
yield gate.promise;
40+
});
41+
});
42+
const acting = start();
43+
flush();
44+
gate.resolve();
45+
await acting;
46+
await new Promise(r => setTimeout(r, 0));
47+
flush();
48+
expect(captured).not.toBeNull();
49+
expect((captured as unknown as Transition)._done).toBe(true);
50+
return captured as unknown as Transition;
51+
}
52+
53+
describe("#3140: completed-transaction stamps", () => {
54+
it("commit clears the pending stamp", async () => {
55+
const gate = deferred();
56+
let start!: () => Promise<unknown>;
57+
createRoot(() => {
58+
start = action(function* () {
59+
yield gate.promise;
60+
});
61+
});
62+
63+
const node = signal(1) as Signal<number>;
64+
// Stage the write in the ambient batch, then open the transaction in the
65+
// same unflushed window: initTransition's batch adoption stamps every
66+
// staged node with it.
67+
setSignal(node, 2);
68+
const acting = start();
69+
expect(node._transition).not.toBeNull();
70+
71+
// Park (incomplete: the action is awaiting), then complete.
72+
flush();
73+
gate.resolve();
74+
await acting;
75+
await new Promise(r => setTimeout(r, 0));
76+
flush();
77+
78+
expect(node._value).toBe(2);
79+
// Pre-fix the stamp survived the commit, pointing at a done transaction.
80+
expect(node._transition).toBeNull();
81+
});
82+
83+
it("a value-equal write to a dead-stamped node does not resurrect the transaction", async () => {
84+
const dead = await completedTransition();
85+
86+
// However a dead stamp survives (the wild race did not minimize), the
87+
// write path must refuse the corpse rather than re-activate it.
88+
const node = signal(5) as Signal<number>;
89+
node._transition = dead;
90+
91+
setSignal(node, 5); // value-equal: bails after the transition re-open ran
92+
// Pre-fix: activeTransition === dead here, and every drain-loop pass that
93+
// rewrote any stamped flag re-armed it — the infinite flush loop.
94+
expect(activeTransition).toBeNull();
95+
flush();
96+
expect(activeTransition).toBeNull();
97+
98+
// A real write is routed to the ambient batch, not the corpse.
99+
const [, setTick] = createSignal(0);
100+
setSignal(node, 6);
101+
setTick(1);
102+
expect(activeTransition).toBeNull();
103+
flush();
104+
expect(node._value).toBe(6);
105+
});
106+
});

scripts/size/.size-limit.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,8 +312,14 @@ module.exports = [
312312
// The #3108 truth-author fix (88fa9d64, optimistic module) plus the
313313
// refresh() quiescence promise (51ffcb9a, settle walk) — this scenario
314314
// retains every store family, so it pays both. Drift, not a regression.
315+
//
316+
// Transaction-lifecycle fixes (2026-08-31): 26.1 -> 26.15 KB, measured at
317+
// 26.12. #3141 (initTransition guarantees a flush) and #3140 (commit
318+
// clears _transition stamps; initTransition refuses a done transaction)
319+
// — ~25 B of scheduler prod code for an ambient-capture fix and a
320+
// prod-hang fix. The other nine budgets absorbed it within headroom.
315321
path: "hydrating-store-app.js",
316-
limit: "26.1 KB",
322+
limit: "26.15 KB",
317323
modifyEsbuildConfig
318324
},
319325
{

0 commit comments

Comments
 (0)