From 546001693636f13700d86c6e93b6086abba2b7d6 Mon Sep 17 00:00:00 2001 From: nosmile99 Date: Tue, 13 Oct 2026 13:03:38 +0000 Subject: [PATCH] fix(process): keep full-budget wait pending when both deadline timers fire early MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-arm in 3e1a366 only fixed the abort timer. wait() itself still resolved "elapsed" when its clamped polling timer fired early at the same monotonic instant as the deadline timer (monotonic 39 / target 40), moving the intermittent failure from the deadline's remaining-budget assertion to the wait's expected "aborted" result. wait() now retains an absolute wait target — the earlier of the requested polling interval and the absolute monotonic deadline — and continues the bounded wait when a timer reports elapsed before that target. Classification is: expired deadline -> "aborted"; elapsed polling target -> "elapsed"; early timer delivery -> re-arm and keep waiting. Caller cancellation, reasons, and timer/listener cleanup are unchanged. Tests: added the "keeps a full-budget wait pending when both timers fire early" probe from the issue review (fails pre-fix: "expected 'elapsed' to be undefined"); strengthened the dispose test with a caller signal and verified it fails if the external removeEventListener is dropped. ProviderDeadline + ProviderProcessOutput + GhidraProvider.behavior + HopperProvider: 64/64 green, stable across 6 runs. check:fast green. --- src/process/ProviderDeadline.test.ts | 39 +++++++++++++++++++++- src/process/ProviderDeadline.ts | 48 ++++++++++++++++++++-------- 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/src/process/ProviderDeadline.test.ts b/src/process/ProviderDeadline.test.ts index 05ee691dcb..43f6673171 100644 --- a/src/process/ProviderDeadline.test.ts +++ b/src/process/ProviderDeadline.test.ts @@ -123,14 +123,51 @@ it("keeps caller cancellation classified without reporting an exhausted budget", } }); +// Probe from issue #2760 review: both the abort timer and the clamped polling +// timer can fire early at the same monotonic instant. The full-budget wait must +// stay pending until the absolute deadline expires. +it("keeps a full-budget wait pending when both timers fire early", async () => { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + let monotonic = 0; + vi.spyOn(performance, "now").mockImplementation(() => monotonic); + const caller = new AbortController(); + const deadline = new ProviderStartupDeadline(40, caller.signal); + let outcome: string | undefined; + const pending = deadline.wait(500).then((result) => { + outcome = result; + return result; + }); + try { + monotonic = 39; + await vi.advanceTimersByTimeAsync(40); + expect(deadline.signal.aborted).toBe(false); + expect(deadline.remainingMs()).toBe(1); + expect(outcome).toBeUndefined(); // currently "elapsed" + monotonic = 40; + await vi.advanceTimersByTimeAsync(1); + await expect(pending).resolves.toBe("aborted"); + expect(deadline.remainingMs()).toBe(0); + } finally { + caller.abort(); + deadline.dispose(); + vi.useRealTimers(); + } +}); + it("releases the timer and listener on dispose with a pending wait", async () => { vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); let monotonic = 0; vi.spyOn(performance, "now").mockImplementation(() => monotonic); - const deadline = new ProviderStartupDeadline(40); + const caller = new AbortController(); + const deadline = new ProviderStartupDeadline(40, caller.signal); try { const pending = deadline.wait(500); deadline.dispose(); + // The external abort listener is removed: caller cancellation after + // dispose must not abort the deadline or classify it as cancelled. + caller.abort(new Error("caller stopped startup")); + expect(deadline.signal.aborted).toBe(false); + expect(deadline.interruption).toBeUndefined(); monotonic = 100; await vi.advanceTimersByTimeAsync(100); expect(deadline.signal.aborted).toBe(false); diff --git a/src/process/ProviderDeadline.ts b/src/process/ProviderDeadline.ts index 0b9390190b..405f0eda7c 100644 --- a/src/process/ProviderDeadline.ts +++ b/src/process/ProviderDeadline.ts @@ -82,20 +82,42 @@ export class ProviderStartupDeadline { return Math.max(0, Math.ceil(this.#deadlineAt - performance.now())); } - /** Wait for at most one polling interval under the shared deadline signal. */ + /** + * Wait for at most one polling interval under the shared deadline signal. + * + * The wait targets the earlier of the requested polling interval and the + * absolute deadline. Node timers can fire up to ~1ms early, and both the + * deadline timer and the clamped polling timer may fire early at the same + * monotonic instant. If a polling timer reports elapsed before the target, + * the wait continues for the residual bounded time instead of resolving — + * the full-budget contract ("aborted" at deadline expiry) holds even when + * every timer in the chain is delivered early. + */ wait(milliseconds: number): Promise { - const budget = this.remainingMs(); - return waitForAbortableDelay( - Math.min(milliseconds, budget), - this.signal, - ).then((result) => { - // A wait clamped to the full budget can resolve at the deadline before - // the re-armed abort timer fires. Report the expired deadline as an - // abort so polling stops at the boundary instead of reporting "elapsed". - if (result === "elapsed" && performance.now() >= this.#deadlineAt) - return "aborted"; - return result; - }); + const startedAt = performance.now(); + // Absolute target: the earlier of the requested polling interval and the + // absolute monotonic deadline. + const targetAt = Math.min(startedAt + milliseconds, this.#deadlineAt); + const poll = (): Promise => { + const budget = this.remainingMs(); + const residual = Math.max(0, targetAt - performance.now()); + return waitForAbortableDelay( + Math.min(residual, budget), + this.signal, + ).then((result) => { + if (result === "aborted") return "aborted"; + const now = performance.now(); + // The monotonic deadline expired: a full-budget wait resolves as an + // abort so polling stops at the boundary. + if (now >= this.#deadlineAt) return "aborted"; + // The polling interval elapsed without expiring the deadline. + if (now >= targetAt) return "elapsed"; + // A timer fired before the absolute target; keep waiting for the + // residual time. The residual strictly decreases, so this converges. + return poll(); + }); + }; + return poll(); } /** Release the deadline timer and external abort listener. */