Skip to content

Commit 90603c5

Browse files
fix: latest()-mode isPending probes are read-order dependent (#3104)
Two probe-mode leaks in the verdict layer. latestRead's mid-tick shadow pull (#2922) recomputed a stale shadow with the probe still live, so the shadow's own read(parent) collected the parent into the probe — the verdict flipped to the held-write answer only when nothing had pulled the shadow current earlier in the tick (reading latest(m) before latest(() => isPending(x)) changed the answer from true to false). And collectPending's companion-verdict reads ran with an outer latest() window still active, dispatching through latestRead and building a shadow OF THE PENDING SIGNAL itself; the next flush's updatePendingSignal wrote that companion-on-companion from inside a recompute and halted dev with the owned-scope write guard. Both sites now suspend the ambient read modes like the companion creation paths always did. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent e27dc29 commit 90603c5

3 files changed

Lines changed: 173 additions & 1 deletion

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+
Fix latest()-mode isPending probes answering differently depending on read order (#3104). Two probe-mode leaks in the verdict layer: latestRead's mid-tick shadow pull recomputed a stale shadow with the probe still live (collecting the parent and flipping the verdict to the held-write answer only when nothing had pulled the shadow earlier in the tick), and the probe's companion-verdict reads ran with an outer latest() window still active, building a shadow of the pending signal itself that later halted dev with the owned-scope write guard when a flush recompute wrote it.

packages/signals/src/core/verdict.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,22 @@ function latestRead<T>(el: Signal<T> | Computed<T>): T {
406406
!(pendingComputed._flags & (REACTIVE_DISPOSED | REACTIVE_ZOMBIE))
407407
) {
408408
markHeap(queue);
409-
prepareComputed(pendingComputed as Computed<unknown>, true);
409+
// Suspend probe collection during the pull (mirrors pendingCheckRead's
410+
// prepare): a probe through latest() answers for the SHADOW — the
411+
// read() dispatch collects it deliberately, so the verdict reflects
412+
// async still in flight for the latest view, not the parent's held
413+
// write. A stale shadow recomputing HERE ran its `read(parent)` with
414+
// the probe still live and collected the parent too, so the verdict
415+
// depended on whether anything had pulled the shadow current earlier
416+
// in the tick (#3104: reading latest(m) flipped a later
417+
// latest(() => isPending(x)) from true to false).
418+
const prevCheck = pendingCheckActive;
419+
setPendingCheckActive(false);
420+
try {
421+
prepareComputed(pendingComputed as Computed<unknown>, true);
422+
} finally {
423+
setPendingCheckActive(prevCheck);
424+
}
410425
}
411426
value = read(pendingComputed);
412427
} catch (e) {
@@ -537,6 +552,17 @@ export function isPending(fn: () => any): boolean {
537552
});
538553
const collectPending = () => {
539554
setPendingCheckActive(false);
555+
// Companion reads are mode-neutral plumbing: under an outer latest()
556+
// (isPending inside a latest window — #3104's memo shape) leaving latest
557+
// mode active dispatched these reads through latestRead, which built a
558+
// SHADOW OF THE PENDING SIGNAL itself. The next updatePendingSignal then
559+
// wrote that companion-on-companion from inside a recompute
560+
// (syncCompanions → setSignal on a shadow created without ownedWrite)
561+
// and halted dev with the owned-scope write guard. The creation paths
562+
// (getLatestValueComputed / getPendingSignal) already suspend both
563+
// modes; this read site must too.
564+
const prevLatest = latestReadActive;
565+
setLatestReadActive(false);
540566
const prevStrictRead = __DEV__ ? strictRead : false;
541567
if (__DEV__) setStrictRead(false);
542568
try {
@@ -548,6 +574,7 @@ export function isPending(fn: () => any): boolean {
548574
});
549575
} finally {
550576
if (__DEV__) setStrictRead(prevStrictRead);
577+
setLatestReadActive(prevLatest);
551578
setPendingCheckActive(true);
552579
}
553580
// A "not pending" verdict that exists only because this reader saw the
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/**
2+
* Regression tests for #3104 — a latest()-mode isPending probe answered
3+
* differently depending on what had been read earlier in the tick.
4+
*
5+
* A probe through latest() answers for the SHADOW companion by design (the
6+
* read() dispatch collects it deliberately): the verdict reflects async
7+
* still in flight for the latest view, not the parent's held write — a
8+
* reader that sees the fresh staged value must not also be told it is
9+
* pending (#2831). But latestRead's untracked mid-tick pull (#2922) ran a
10+
* stale shadow's recompute with the probe still live, so the shadow's own
11+
* `read(parent)` collected the parent too and flipped the verdict to the
12+
* parent's held-write answer. Whether the shadow was stale depended on what
13+
* had pulled it earlier in the tick — reading `latest(m)` before the probe
14+
* flipped a later `latest(() => isPending(x))` from true to false.
15+
*/
16+
import { describe, expect, it } from "vitest";
17+
import {
18+
createMemo,
19+
createRenderEffect,
20+
createRoot,
21+
createSignal,
22+
flush,
23+
isPending,
24+
latest
25+
} from "../src/index.js";
26+
27+
describe("latest-mode probe order independence (#3104)", () => {
28+
function stage(withExternalLatestRead: boolean) {
29+
const [count, setCount] = createSignal(0);
30+
// Ownerless, like the issue's setTimeout-created memo.
31+
const m1 = createMemo(() => latest(() => isPending(count)));
32+
33+
expect(m1()).toBe(false);
34+
expect(isPending(count)).toBe(false);
35+
36+
setCount(v => v + 1);
37+
38+
const reads = {
39+
m1Internal: m1(),
40+
m1External: withExternalLatestRead ? latest(m1) : undefined,
41+
direct: isPending(count),
42+
latestProbe: latest(() => isPending(count))
43+
};
44+
flush();
45+
return { reads, settled: { m1: m1(), direct: isPending(count) } };
46+
}
47+
48+
it("latest(() => isPending(x)) answers the same whether or not latest(m) was read before it", () => {
49+
const withRead = stage(true);
50+
const without = stage(false);
51+
52+
// The heisenberg: these two used to disagree (false vs true).
53+
expect(withRead.reads.latestProbe).toBe(without.reads.latestProbe);
54+
55+
// The designed pre-flush answers for a plain staged write: the direct
56+
// probe reports the held write; every latest-flavored probe saw the
57+
// fresh value in the latest view, so it must not also report pending.
58+
expect(withRead.reads).toEqual({
59+
m1Internal: false, // memo created pre-write, no flush seen yet (#3078)
60+
m1External: false,
61+
direct: true,
62+
latestProbe: false
63+
});
64+
expect(without.reads).toMatchObject({
65+
m1Internal: false,
66+
direct: true,
67+
latestProbe: false
68+
});
69+
70+
// Post-flush the plain write has committed everywhere.
71+
expect(withRead.settled).toEqual({ m1: false, direct: false });
72+
expect(without.settled).toEqual({ m1: false, direct: false });
73+
});
74+
75+
// isPending probes running inside a latest() window (mizulu's memo is
76+
// exactly `latest(() => isPending(count))`) left latest mode active during
77+
// the probe's companion-verdict reads, so `read(getPendingSignal(...))`
78+
// dispatched through latestRead and built a shadow OF THE PENDING SIGNAL.
79+
// The next flush's updatePendingSignal wrote that companion-on-companion
80+
// from inside a recompute and halted dev with the owned-scope write guard.
81+
it("isPending inside a latest window survives subsequent flushes and stays coherent", async () => {
82+
const tick = async () => {
83+
await new Promise(r => setTimeout(r, 0));
84+
flush();
85+
};
86+
const log: string[] = [];
87+
let setA!: (v: number) => void;
88+
const resolvers: Array<() => void> = [];
89+
let dispose!: () => void;
90+
91+
createRoot(d => {
92+
dispose = d;
93+
const [a, set] = createSignal(0);
94+
setA = set;
95+
const double = createMemo(async () => {
96+
const x = a();
97+
await new Promise<void>(r => {
98+
resolvers.push(r);
99+
});
100+
return x * 2;
101+
});
102+
createRenderEffect(
103+
() => {
104+
try {
105+
return double();
106+
} catch {
107+
return "THROWN";
108+
}
109+
},
110+
() => {}
111+
);
112+
createRenderEffect(
113+
() =>
114+
`plain=${isPending(() => a())} latestP=${latest(() =>
115+
isPending(() => a())
116+
)} latestPm=${latest(() => isPending(() => double()))}`,
117+
v => {
118+
log.push(v);
119+
}
120+
);
121+
});
122+
flush();
123+
resolvers.splice(0).forEach(r => r());
124+
await tick(); // used to throw REACTIVE_WRITE_IN_OWNED_SCOPE here
125+
expect(log[log.length - 1]).toBe("plain=false latestP=false latestPm=false");
126+
127+
// Refetch in flight: the direct probe reports the held input; the
128+
// latest-window probe of the SIGNAL saw the staged value (pairing rule,
129+
// #2831); the latest-window probe of the async MEMO stays pending — its
130+
// answer is still computing, so there is nothing fresher to see (#3028).
131+
setA(1);
132+
flush();
133+
expect(log[log.length - 1]).toBe("plain=true latestP=false latestPm=true");
134+
135+
resolvers.splice(0).forEach(r => r());
136+
await tick();
137+
expect(log[log.length - 1]).toBe("plain=false latestP=false latestPm=false");
138+
dispose();
139+
});
140+
});

0 commit comments

Comments
 (0)