Skip to content

Commit ff0a5aa

Browse files
committed
fix(vnext): isolate terminal intent timer generations
1 parent 2201cc7 commit ff0a5aa

2 files changed

Lines changed: 116 additions & 17 deletions

File tree

src/vnext/__tests__/session.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3916,6 +3916,94 @@ describe("session coverage hardening", () => {
39163916
}
39173917
});
39183918

3919+
it("does not let a stale intent callback erase a newer timer", async () => {
3920+
const nativeSetTimeout = globalThis.setTimeout;
3921+
const nativeClearTimeout = globalThis.clearTimeout;
3922+
const callbacks: Array<() => void> = [];
3923+
const handles: object[] = [];
3924+
const cleared: number[] = [];
3925+
Object.defineProperty(globalThis, "setTimeout", {
3926+
configurable: true,
3927+
value: (
3928+
callback: TimerHandler,
3929+
delay?: number,
3930+
...arguments_: unknown[]
3931+
) => {
3932+
if (delay !== 1_000) {
3933+
return nativeSetTimeout(callback, delay, ...arguments_);
3934+
}
3935+
if (typeof callback !== "function") {
3936+
throw new Error("Intent timer callback must be a function");
3937+
}
3938+
callbacks.push(() => {
3939+
Reflect.apply(callback, undefined, arguments_);
3940+
});
3941+
const handle = Object.freeze({ id: handles.length });
3942+
handles.push(handle);
3943+
return handle;
3944+
},
3945+
writable: true,
3946+
});
3947+
Object.defineProperty(globalThis, "clearTimeout", {
3948+
configurable: true,
3949+
value: (handle: unknown) => {
3950+
const index = handles.findIndex(
3951+
(candidate) => candidate === handle,
3952+
);
3953+
if (index >= 0) {
3954+
cleared.push(index);
3955+
} else {
3956+
Reflect.apply(nativeClearTimeout, globalThis, [handle]);
3957+
}
3958+
},
3959+
writable: true,
3960+
});
3961+
try {
3962+
const service = catalogService({
3963+
id: "stale-intent-timer",
3964+
search: async () => ({
3965+
epoch: { generation: 0, token: "loading" },
3966+
status: "loading",
3967+
}),
3968+
});
3969+
const session = service.openDocument({
3970+
context: {
3971+
catalog: { scope: "connection:stale-intent-timer" },
3972+
dialect: "duckdb",
3973+
engine: "local",
3974+
},
3975+
text: "SELECT * FROM ",
3976+
});
3977+
await session.complete({
3978+
position: 14,
3979+
trigger: { kind: "invoked" },
3980+
});
3981+
await session.complete({
3982+
position: 14,
3983+
trigger: { kind: "invoked" },
3984+
});
3985+
expect(callbacks).toHaveLength(2);
3986+
expect(cleared).toContain(0);
3987+
3988+
callbacks[0]?.();
3989+
session.dispose();
3990+
3991+
expect(cleared).toContain(1);
3992+
service.dispose();
3993+
} finally {
3994+
Object.defineProperty(globalThis, "setTimeout", {
3995+
configurable: true,
3996+
value: nativeSetTimeout,
3997+
writable: true,
3998+
});
3999+
Object.defineProperty(globalThis, "clearTimeout", {
4000+
configurable: true,
4001+
value: nativeClearTimeout,
4002+
writable: true,
4003+
});
4004+
}
4005+
});
4006+
39194007
it("consumes settlement racing the soft response timer", async () => {
39204008
let resolveSearch:
39214009
| ((

src/vnext/session.ts

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ interface SessionChangeSubscription {
101101
readonly listener: (event: SqlSessionChangeEvent) => void;
102102
}
103103

104+
interface SessionTimerCell {
105+
active: boolean;
106+
handle: ReturnType<typeof setTimeout> | undefined;
107+
}
108+
104109
interface CompletionConfiguration {
105110
readonly catalogResponseBudgetMs: number;
106111
}
@@ -757,10 +762,7 @@ export class DefaultSqlDocumentSession<Context extends SqlDocumentContext>
757762
#refreshIntent: CompletionRequestState | null = null;
758763
#snapshot: SessionSnapshot<Context>;
759764
#statementIndexCache: StatementIndexCache | null = null;
760-
#terminalIntentTimer:
761-
| ReturnType<typeof setTimeout>
762-
| undefined;
763-
#terminalIntentTimerScheduled = false;
765+
#terminalIntentTimer: SessionTimerCell | null = null;
764766
#updating = false;
765767

766768
constructor(
@@ -834,11 +836,11 @@ export class DefaultSqlDocumentSession<Context extends SqlDocumentContext>
834836
}
835837

836838
#clearTerminalIntent(): void {
837-
if (!this.#terminalIntentTimerScheduled) return;
838-
this.#terminalIntentTimerScheduled = false;
839-
const timer = this.#terminalIntentTimer;
840-
this.#terminalIntentTimer = undefined;
841-
clearTimeout(timer);
839+
const cell = this.#terminalIntentTimer;
840+
if (!cell) return;
841+
this.#terminalIntentTimer = null;
842+
cell.active = false;
843+
clearTimeout(cell.handle);
842844
}
843845

844846
#dispatchChange(
@@ -1333,15 +1335,24 @@ export class DefaultSqlDocumentSession<Context extends SqlDocumentContext>
13331335
if (outcome.response.status === "loading") {
13341336
remainingIntentLeaseMs =
13351337
TERMINAL_LOADING_INTENT_LEASE_MS;
1336-
this.#terminalIntentTimerScheduled = true;
1337-
const timer = setTimeout(() => {
1338-
this.#terminalIntentTimerScheduled = false;
1339-
this.#terminalIntentTimer = undefined;
1338+
const cell: SessionTimerCell = {
1339+
active: true,
1340+
handle: undefined,
1341+
};
1342+
this.#terminalIntentTimer = cell;
1343+
const handle = setTimeout(() => {
1344+
if (!cell.active) return;
1345+
cell.active = false;
1346+
if (this.#terminalIntentTimer === cell) {
1347+
this.#terminalIntentTimer = null;
1348+
}
13401349
}, remainingIntentLeaseMs);
1341-
if (this.#terminalIntentTimerScheduled) {
1342-
this.#terminalIntentTimer = timer;
1343-
} else {
1344-
clearTimeout(timer);
1350+
cell.handle = handle;
1351+
if (
1352+
!cell.active ||
1353+
this.#terminalIntentTimer !== cell
1354+
) {
1355+
clearTimeout(handle);
13451356
}
13461357
}
13471358
}

0 commit comments

Comments
 (0)