From 6183544e8b42b65feaf4f674f860b1fb3b0c95eb Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 16 Oct 2026 06:06:11 +0800 Subject: [PATCH] fix(javascript): select timer handles at cancellation reads Use the shared binding-at-read owner instead of grouping schedules across the whole binding lifetime. Match selected initializer nodes, retain uncertain origins as candidate links, and preserve immutable closure captures. Adapt the diagnosis in PR #3794 by @Xinglan233, whose source commit was authored by Codex. Verify stale writes, later schedules, replacements and CLI graph links. Refs #3358, #3794 Co-authored-by: Codex --- .../javascriptSemanticAnalysis.timers.test.ts | 89 +++++++++++++++++++ .../javascriptSemanticAsyncEffects.ts | 74 +++++++++++---- .../javascriptTimerWritesCli.test.ts | 54 +++++++++++ 3 files changed, 198 insertions(+), 19 deletions(-) create mode 100644 src/domain/javascript/javascriptSemanticAnalysis.timers.test.ts create mode 100644 tests/acceptance/applications/javascriptTimerWritesCli.test.ts diff --git a/src/domain/javascript/javascriptSemanticAnalysis.timers.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.timers.test.ts new file mode 100644 index 0000000000..0df828281b --- /dev/null +++ b/src/domain/javascript/javascriptSemanticAnalysis.timers.test.ts @@ -0,0 +1,89 @@ +import { expect, it } from "vitest"; + +import { analyzeJavaScriptSemantics } from "./javascriptSemanticAnalysis.js"; + +const timerOperations = (source: string) => + analyzeJavaScriptSemantics(source).timerOperations; + +it("does not link cancellation to a timer after its handle is cleared", () => { + const operations = timerOperations(` + let handle = setTimeout(work, 10); + handle = null; + clearTimeout(handle); + `); + + expect(operations.at(-1)).toMatchObject({ + kind: "cancel", + linkedTimerId: null, + resolution: "unresolved", + }); +}); + +it("links cancellation to the latest preceding timer assignment", () => { + const operations = timerOperations(` + let handle = setTimeout(first, 10); + handle = setTimeout(second, 20); + clearTimeout(handle); + `); + + expect(operations[2]).toMatchObject({ + kind: "cancel", + linkedTimerId: operations[1]?.timerId, + resolution: "complete", + }); +}); + +it("does not link cancellation to a timer scheduled later", () => { + const operations = timerOperations(` + let handle; + clearTimeout(handle); + handle = setTimeout(work, 10); + `); + + expect(operations[0]).toMatchObject({ + kind: "cancel", + linkedTimerId: null, + resolution: "unresolved", + }); +}); + +it("preserves an immutable timer handle captured by a closure", () => { + const operations = timerOperations(` + const handle = setTimeout(work, 10); + const cancel = () => clearTimeout(handle); + `); + + expect(operations[1]).toMatchObject({ + kind: "cancel", + linkedTimerId: operations[0]?.timerId, + resolution: "complete", + }); +}); + +it("keeps cancellation uncertain after a conditional handle replacement", () => { + const operations = timerOperations(` + let handle = setTimeout(work, 10); + if (condition) handle = null; + clearTimeout(handle); + `); + + expect(operations.at(-1)).toMatchObject({ + kind: "cancel", + linkedTimerId: operations[0]?.timerId, + resolution: "partial", + }); +}); + +it("keeps cancellation uncertain across a mutable closure capture", () => { + const operations = timerOperations(` + let handle = setTimeout(work, 10); + const cancel = () => clearTimeout(handle); + handle = null; + `); + + expect(operations[1]).toMatchObject({ + kind: "cancel", + linkedTimerId: operations[0]?.timerId, + resolution: "partial", + }); +}); diff --git a/src/domain/javascript/javascriptSemanticAsyncEffects.ts b/src/domain/javascript/javascriptSemanticAsyncEffects.ts index a34bfb7500..97a2d03b34 100644 --- a/src/domain/javascript/javascriptSemanticAsyncEffects.ts +++ b/src/domain/javascript/javascriptSemanticAsyncEffects.ts @@ -13,6 +13,7 @@ import { type JavaScriptSemanticAnalysisState, type JavaScriptSemanticBindingState, } from "./javascriptSemanticState.js"; +import { semanticBindingAtRead } from "./javascriptSemanticMutationInitializers.js"; import { traverseJavaScriptAst } from "./javascriptSemanticTraversal.js"; import { range } from "./javascriptStaticAnalysisHelpers.js"; import { @@ -75,6 +76,10 @@ interface TimerCandidate { readonly projectionPath: readonly (string | number | null)[]; readonly resolution: "complete" | "partial"; }[]; + readonly handleRead: { + readonly origins: readonly t.Node[]; + readonly complete: boolean; + } | null; } interface TimerCollectionInput { @@ -219,6 +224,38 @@ const collectTimer = (input: TimerCollectionInput): void => { ancestors, state, ); + const handle = + kind === "cancel" && t.isIdentifier(node.arguments[0]) + ? resolveSemanticBindingState( + state, + node.arguments[0], + node.arguments[0].name, + ) + : undefined; + const handleRead = + handle === undefined || !t.isIdentifier(node.arguments[0]) + ? null + : (() => { + const snapshot = semanticBindingAtRead( + handle, + node.arguments[0], + state.parentsByNode, + ); + // A const handle has the same value inside a later closure even + // when the source positions belong to different function bodies. + const stableClosureCapture = + !handle.mutable && + handle.initializers.length === 1 && + handle.referenceInitializers.length === 0; + const origins = [ + ...snapshot.binding.initializers, + ...snapshot.binding.referenceInitializers, + ]; + return { + origins: origins.map(({ node: origin }) => origin), + complete: snapshot.status === "ordered" || stableClosureCapture, + }; + })(); output.push({ node, method, @@ -236,32 +273,29 @@ const collectTimer = (input: TimerCollectionInput): void => { ), ) : [], + handleRead, }); }; const resolveTimers = ( candidates: readonly TimerCandidate[], ): JavaScriptSemanticTimerOperation[] => { - const schedulesByBinding = new Map(); - for (const candidate of candidates) { - if (candidate.kind !== "schedule") continue; - const ids = [ - ...(candidate.handleBindingId === null - ? [] - : [candidate.handleBindingId]), - ...candidate.projectedResultBindings.map(({ bindingId }) => bindingId), - ]; - for (const id of ids) { - const existing = schedulesByBinding.get(id) ?? []; - existing.push(candidate); - schedulesByBinding.set(id, existing); - } - } + const schedules = new Map( + candidates + .filter(({ kind }) => kind === "schedule") + .map((candidate) => [candidate.node, candidate] as const), + ); return candidates.map((candidate) => { + // Unordered reads retain possible initializers as candidate edges. Only an + // ordered or immutable read can establish one complete cancellation link. const linked = - candidate.kind === "cancel" && candidate.handleBindingId !== null - ? (schedulesByBinding.get(candidate.handleBindingId) ?? []) - : []; + candidate.handleRead?.origins.flatMap((origin) => { + const schedule = schedules.get(origin); + return schedule === undefined ? [] : [schedule]; + }) ?? []; + const incompleteOrigins = + candidate.handleRead !== null && + candidate.handleRead.origins.length !== linked.length; return { timerId: timerId(candidate), kind: candidate.kind, @@ -279,11 +313,13 @@ const resolveTimers = ( candidate.kind === "schedule" ? "complete" : linked.length === 1 && + candidate.handleRead?.complete && + !incompleteOrigins && !linked[0]?.projectedResultBindings.some( ({ bindingId }) => bindingId === candidate.handleBindingId, ) ? "complete" - : linked.length > 1 + : linked.length > 0 ? "partial" : "unresolved", }; diff --git a/tests/acceptance/applications/javascriptTimerWritesCli.test.ts b/tests/acceptance/applications/javascriptTimerWritesCli.test.ts new file mode 100644 index 0000000000..113ec586a8 --- /dev/null +++ b/tests/acceptance/applications/javascriptTimerWritesCli.test.ts @@ -0,0 +1,54 @@ +import { expect } from "vitest"; + +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; +import { analyzeJavaScriptDirectory } from "../../support/cli/javascriptApplication.js"; +import { writeFixtureFiles } from "../../support/javascriptApplicationFixture.js"; + +cliTest( + "links cancellation to the timer handle reaching its read through the CLI", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-timer-reaching-write-"); + await writeFixtureFiles(root, { + "app.mjs": ` + function cleared() { + let handle = setTimeout(work, 10); + handle = null; + clearTimeout(handle); + } + function latest() { + let handle = setTimeout(work, 20); + handle = setTimeout(work, 30); + clearTimeout(handle); + } + function conditional() { + let handle = setTimeout(work, 40); + if (condition) handle = null; + clearTimeout(handle); + } + `, + }); + const result = await analyzeJavaScriptDirectory("CLI", cli, root); + expect(result.statistics.parse_failures).toBe(0); + const graph = result.semantic_graph; + expect(graph).not.toBeNull(); + if (graph === null) throw new Error("Expected a semantic graph"); + const cancellationTargets = graph.relations + .filter(({ relation }) => relation === "cancels-timer") + .map(({ resolution, target_node_id }) => ({ + resolution, + delay: graph.nodes.find(({ node_id }) => node_id === target_node_id) + ?.properties.delay_milliseconds, + })); + expect(cancellationTargets).toEqual( + expect.arrayContaining([ + { resolution: "resolved", delay: 30 }, + { resolution: "candidate", delay: 40 }, + ]), + ); + expect(cancellationTargets).toHaveLength(2); + expect( + graph.unknowns.filter(({ family }) => family === "timer"), + ).toHaveLength(2); + }, +);