Skip to content

Commit f0bdfad

Browse files
fix: keep scheduler state consistent when an effect throws
Two scheduler-hygiene fixes that stop a thrown effect from corrupting shared scheduler state (the error still propagates): - flush(fn) now decrements syncDepth in a finally, so a throwing drain can no longer leak the counter and leave schedule() permanently unable to queue a microtask. - adjustHeight routes each height-adjusted subscriber to the queue that matches its own zombie flag, mirroring the post-recompute path. This avoids parking a zombie in dirtyQueue (or a live node in zombieQueue), the same flag/queue invariant violation behind the #2759 livelock. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 31be242 commit f0bdfad

5 files changed

Lines changed: 59 additions & 4 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+
Route height-adjusted subscribers to the queue that matches their own zombie flag in `adjustHeight`, mirroring the post-recompute height-adjust path. Inserting into the currently running heap unconditionally could park a zombie node in `dirtyQueue` (or a live node in `zombieQueue`), breaking the flag/queue invariant `deleteFromHeap` relies on — the same corruption class behind the #2759 livelock, reachable through a different trigger.
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+
`flush(fn)` now restores its sync-depth counter when the drain throws. An effect that throws inside a synchronous flush scope previously leaked the counter, which left `schedule()` permanently unable to queue a microtask and silently froze all later reactivity. Balancing it in a `finally` keeps the scheduler usable after the error propagates.

packages/solid-signals/src/core/heap.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import {
44
REACTIVE_IN_HEAP,
55
REACTIVE_IN_HEAP_HEIGHT,
66
REACTIVE_MANUAL_WRITE,
7-
REACTIVE_RECOMPUTING_DEPS
7+
REACTIVE_RECOMPUTING_DEPS,
8+
REACTIVE_ZOMBIE
89
} from "./constants.js";
10+
import { dirtyQueue, zombieQueue } from "./scheduler.js";
911
import type { Computed, FirewallSignal, Root } from "./types.js";
1012

1113
export interface Heap {
@@ -130,7 +132,12 @@ function adjustHeight(el: Computed<unknown>, heap: Heap) {
130132
if (el._height !== newHeight) {
131133
el._height = newHeight;
132134
for (let s = el._subs; s !== null; s = s._nextSub) {
133-
insertIntoHeapHeight(s._sub, heap);
135+
// Route each subscriber by its own zombie flag, mirroring the
136+
// post-recompute height-adjust path. Inserting into the running `heap`
137+
// unconditionally can park a zombie in `dirtyQueue` (or a live node in
138+
// `zombieQueue`), breaking the flag/queue invariant `deleteFromHeap`
139+
// relies on — the same corruption class as #2759.
140+
insertIntoHeapHeight(s._sub, s._sub._flags & REACTIVE_ZOMBIE ? zombieQueue : dirtyQueue);
134141
}
135142
}
136143
}

packages/solid-signals/src/core/scheduler.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -658,8 +658,13 @@ export function flush<T>(fn?: () => T): T | void {
658658
try {
659659
return fn();
660660
} finally {
661-
flush();
662-
syncDepth--;
661+
// Decrement even if the drain throws (a throwing effect): a leaked
662+
// syncDepth would stop `schedule()` from ever queuing a microtask again.
663+
try {
664+
flush();
665+
} finally {
666+
syncDepth--;
667+
}
663668
}
664669
}
665670
if (globalQueue._running) {

packages/solid-signals/tests/flush.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,39 @@ import { createEffect, createRoot, createSignal, flush } from "../src/index.js";
22

33
afterEach(() => flush());
44

5+
describe("scheduler hygiene around throwing effects", () => {
6+
it("balances syncDepth when an effect throws inside flush(fn)", async () => {
7+
const [a, setA] = createSignal(0);
8+
const [c, setC] = createSignal(0);
9+
const seenC: number[] = [];
10+
11+
createRoot(() => {
12+
// Throws without writing a signal, so the drain leaves no pending work —
13+
// this isolates the syncDepth leak from any stuck-`scheduled` symptom.
14+
createEffect(a, v => {
15+
if (v === 1) throw new Error("boom");
16+
});
17+
createEffect(c, v => {
18+
seenC.push(v);
19+
});
20+
});
21+
flush();
22+
// Drain the microtask queued by effect creation; a leftover microtask would
23+
// otherwise mask a leaked syncDepth by draining the work for us.
24+
await Promise.resolve();
25+
seenC.length = 0;
26+
27+
expect(() => flush(() => setA(1))).toThrow("boom");
28+
29+
// A clean write must still schedule a microtask. If the throw above leaked
30+
// syncDepth, `schedule()` would never queue one again and this would stall.
31+
setC(3);
32+
await Promise.resolve();
33+
await Promise.resolve();
34+
expect(seenC).toEqual([3]);
35+
});
36+
});
37+
538
it("should batch updates", () => {
639
const [$x, setX] = createSignal(10);
740
const effect = vi.fn();

0 commit comments

Comments
 (0)