Skip to content

Commit fa13761

Browse files
fix: close superseded async-iterable flights at supersede time (#3122)
The iterator close was registered only as an owner cleanup, and a recompute whose previous-run disposal rides the zombie-deferred channel drains it at commitPendingNode — which a verdict-held write defers until the SUPERSEDING flight settles, so the stale iterator ran to completion (its landing discarded by flight identity) and return() fired only after the new answer arrived. Iterator close is the cancellation hook for anything resource-shaped behind a stream, so a _flightTeardown slot on the extension now fires at recompute's _inFlight release, keyed to flight identity; the owner-cleanup registration stays as the death backstop (close is idempotent). Conscious treeshake-floor bump: +96B, measured 21,331. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 90603c5 commit fa13761

6 files changed

Lines changed: 210 additions & 2 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+
Close superseded async-iterable flights at supersede time (#3122). The iterator close was registered only as an owner cleanup, and a recompute whose disposal rides the zombie-deferred channel drains it at commitPendingNode — which a verdict-held write defers until the SUPERSEDING flight settles, leaving the stale iterator running to completion. Iterator close is the cancellation hook for resource-shaped streams (fibers, sockets, subscriptions), so the flight teardown now also fires at recompute's `_inFlight` release, keyed to flight identity; the owner cleanup stays as the death backstop.

packages/signals/src/core/async.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,15 @@ export function isThenable<T>(value: T | PromiseLike<T>): value is PromiseLike<T
242242
);
243243
}
244244

245+
/** Fire and clear a node's iterator-flight cancellation hook (#3122). */
246+
export function releaseFlightTeardown(el: Computed<any>): void {
247+
const teardown = el._x?._flightTeardown;
248+
if (teardown != null) {
249+
el._x!._flightTeardown = null;
250+
teardown();
251+
}
252+
}
253+
245254
export function handleAsync<T>(
246255
el: Computed<T>,
247256
result: T | PromiseLike<T> | AsyncIterable<T>,
@@ -286,6 +295,11 @@ export function handleAsync<T>(
286295
throw new Error(message);
287296
}
288297

298+
// Flight replacement relies on recompute's supersede release for iterator
299+
// teardown (#3122): every handleAsync call — including the projection
300+
// self-registration — runs during a recompute of `el`, which has already
301+
// fired _flightTeardown. A future non-recompute registration path must
302+
// release it here before overwriting _inFlight.
289303
ext(el)._inFlight = result as PromiseLike<T> | AsyncIterable<T>;
290304
let syncValue: T;
291305

@@ -499,6 +513,11 @@ export function handleAsync<T>(
499513
} catch {}
500514
};
501515
registerClose ? registerClose(close) : cleanup(close);
516+
// Flight-identity cancellation (#3122): the registration above is the
517+
// owner-death backstop, but its disposal list can be zombie-deferred
518+
// until the SUPERSEDING flight settles. The teardown slot fires at the
519+
// _inFlight release sites so supersede stops this stream immediately.
520+
ext(el)._flightTeardown = close;
502521

503522
// Release check before each next pull: an unobserved lazy node must tear
504523
// down (its close above runs via disposal, closing the iterator) instead

packages/signals/src/core/core.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
handleAsync,
44
notifyStatus,
55
parkLoadingWindow,
6+
releaseFlightTeardown,
67
settleErroredDependents
78
} from "./async.js";
89
import {
@@ -206,7 +207,15 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
206207
if (el._transition && (!isEffect || activeTransition) && activeTransition !== el._transition)
207208
globalQueue.initTransition(el._transition);
208209
deleteFromHeap(el, queueFor(el));
209-
if (el._x !== null) el._x._inFlight = null;
210+
if (el._x !== null) {
211+
el._x._inFlight = null;
212+
// Supersede is where an iterator flight dies (#3122): close it now.
213+
// Its cleanup(close) registration may sit in a zombie-deferred
214+
// disposal list that a held transition only drains when the
215+
// SUPERSEDING flight settles — cancellation must not wait for the
216+
// work that replaced it. Idempotent with the cleanup-channel close.
217+
releaseFlightTeardown(el);
218+
}
210219
// Tracked effects run after finalizePureQueue, so dispose immediately instead of deferring
211220
if (el._transition || isEffect === EFFECT_TRACKED) disposeChildren(el);
212221
else if (el._firstChild !== null || el._disposal !== null) {
@@ -646,6 +655,7 @@ export function ext(el: { _x: NodeExtension | null }): NodeExtension {
646655
_parentSource: undefined,
647656
_affectsCount: 0,
648657
_inFlight: null,
658+
_flightTeardown: null,
649659
_error: undefined,
650660
_blocked: undefined,
651661
_pendingSources: undefined,

packages/signals/src/core/types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,13 @@ export interface NodeExtension {
8585
*/
8686
_affectsCount: number;
8787
_inFlight: PromiseLike<any> | AsyncIterable<any> | null;
88+
/** Cancellation for the CURRENT iterator flight (#3122): closes the
89+
* iterator (`it.return()`), idempotent. Fired at the sites that release
90+
* `_inFlight` so a superseded stream stops at supersede time — its owner
91+
* cleanup registration may ride the zombie-disposal channel, which a held
92+
* transition defers until the SUPERSEDING flight settles. Null for plain
93+
* promise flights (no cancellation hook exists). */
94+
_flightTeardown: (() => void) | null;
8895
_error: unknown;
8996
_blocked: boolean | undefined;
9097
_pendingSources: Set<Computed<any>> | undefined;
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/**
2+
* Regression tests for #3122 — superseded async-iterable flights must close
3+
* (`it.return()`) at supersede time, not when the SUPERSEDING flight settles.
4+
*
5+
* The iterator close is registered as an owner cleanup, and a recompute
6+
* whose disposal rides the zombie-deferred channel only drains it at
7+
* commitPendingNode — which a verdict-held write defers until the new
8+
* flight lands. Iterator close is the cancellation hook for anything
9+
* resource-shaped behind an async iterable (fibers, sockets, subscriptions),
10+
* so the flight teardown now also fires at the `_inFlight` release site in
11+
* recompute, keyed to flight identity. The owner-cleanup registration stays
12+
* as the death backstop (close is idempotent).
13+
*/
14+
import { describe, expect, it } from "vitest";
15+
import {
16+
createEffect,
17+
createLoadingBoundary,
18+
createMemo,
19+
createRoot,
20+
createSignal,
21+
flush,
22+
isPending,
23+
latest
24+
} from "../src/index.js";
25+
26+
function trackedSource(label: string, events: string[]) {
27+
let land!: () => void;
28+
const source: AsyncIterable<string> & { land: () => void } = {
29+
land: () => land(),
30+
[Symbol.asyncIterator]() {
31+
events.push(`open ${label}`);
32+
let done = false;
33+
return {
34+
next: () =>
35+
new Promise<IteratorResult<string>>(resolve => {
36+
land = () => {
37+
if (done) return resolve({ done: true, value: undefined });
38+
done = true;
39+
events.push(`land ${label}`);
40+
resolve({ done: false, value: label });
41+
};
42+
}),
43+
return: async () => {
44+
events.push(`close ${label}`);
45+
done = true;
46+
return { done: true as const, value: undefined };
47+
}
48+
};
49+
}
50+
};
51+
return source;
52+
}
53+
54+
const microtasks = async () => {
55+
await Promise.resolve();
56+
await Promise.resolve();
57+
await Promise.resolve();
58+
flush();
59+
};
60+
61+
describe("superseded iterator close timing (#3122)", () => {
62+
it("closes the stale iterator at supersede even under a loading boundary with isPending", async () => {
63+
const events: string[] = [];
64+
const sources = new Map<string, ReturnType<typeof trackedSource>>();
65+
let setQ!: (v: string) => void;
66+
let dispose!: () => void;
67+
68+
createRoot(d => {
69+
dispose = d;
70+
const [q, set] = createSignal("");
71+
setQ = set;
72+
const m = createMemo(() => {
73+
const v = q();
74+
if (!v) return [] as unknown as AsyncIterable<string>;
75+
const src = trackedSource(v, events);
76+
sources.set(v, src);
77+
return src;
78+
});
79+
// The issue's failing shape: boundary content reading BOTH isPending
80+
// and latest over the same source.
81+
const view = createLoadingBoundary(
82+
() => {
83+
try {
84+
const pending = isPending(() => m());
85+
return { pending, value: latest(() => m()) };
86+
} catch {
87+
return undefined;
88+
}
89+
},
90+
() => "loading-fallback"
91+
);
92+
createEffect(
93+
() => view(),
94+
() => {}
95+
);
96+
});
97+
flush();
98+
99+
setQ("a");
100+
flush();
101+
expect(events).toEqual(["open a"]);
102+
103+
// Supersede before 'a' lands: the stale iterator must close NOW, not
104+
// after 'ab' settles.
105+
setQ("ab");
106+
flush();
107+
await microtasks();
108+
expect(events).toEqual(["open a", "close a", "open ab"]);
109+
110+
sources.get("ab")!.land();
111+
await microtasks();
112+
expect(events).toEqual(["open a", "close a", "open ab", "land ab"]);
113+
114+
// The closed stale flight can no longer land a value.
115+
sources.get("a")!.land();
116+
await microtasks();
117+
expect(events).toEqual(["open a", "close a", "open ab", "land ab"]);
118+
dispose();
119+
});
120+
121+
it("closes the stale iterator at supersede for a plain reader too", async () => {
122+
const events: string[] = [];
123+
const sources = new Map<string, ReturnType<typeof trackedSource>>();
124+
let setQ!: (v: string) => void;
125+
let dispose!: () => void;
126+
127+
createRoot(d => {
128+
dispose = d;
129+
const [q, set] = createSignal("a");
130+
setQ = set;
131+
const m = createMemo(() => {
132+
const v = q();
133+
const src = trackedSource(v, events);
134+
sources.set(v, src);
135+
return src;
136+
});
137+
createEffect(
138+
() => {
139+
try {
140+
return latest(() => m());
141+
} catch {
142+
return undefined;
143+
}
144+
},
145+
() => {}
146+
);
147+
});
148+
flush();
149+
expect(events).toEqual(["open a"]);
150+
151+
setQ("b");
152+
flush();
153+
await microtasks();
154+
expect(events).toEqual(["open a", "close a", "open b"]);
155+
156+
sources.get("b")!.land();
157+
await microtasks();
158+
expect(events).toEqual(["open a", "close a", "open b", "land b"]);
159+
dispose();
160+
});
161+
});

packages/signals/tests/treeshake.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,13 @@ describe("pay-for-use tree-shaking (#2883)", () => {
120120
// settle seam; the registry itself lives in core/quiescence.ts and
121121
// shakes out with refresh()). Paid for by converting three
122122
// `slot !== null && slot(...)` gates to `slot?.(...)`.
123-
expect(minifiedBytes).toBeLessThan(21_250);
123+
// CONSCIOUS BUMP (2026-08-31): +~96B for flight-identity iterator
124+
// cancellation (#3122) — the `_flightTeardown` ext slot, its
125+
// registration in consumeIterator, and recompute's supersede release.
126+
// Core-retained by necessity: supersede happens in recompute, and the
127+
// async-iterable machinery is already part of the memo floor. Measured
128+
// at 21,331 post-change.
129+
expect(minifiedBytes).toBeLessThan(21_400);
124130
});
125131

126132
it("plain stores shed the verdict layer, affects, boundaries, and map", async () => {

0 commit comments

Comments
 (0)