From 44d232fccb2532d2a68fffc10c9dd426d9df3aa7 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 9 Nov 2026 06:07:27 +0800 Subject: [PATCH] fix(providers): retry live waiters after abandoned startup drains (#9467) * fix(providers): retry live waiters after abandoned startup drains * test(providers): bound startup lifecycle acknowledgements --- .../binary/DeferredAnalysisClient.test.ts | 157 ++++++++++++++++++ src/ghidra/GhidraProvider.behavior.test.ts | 105 +++++++++++- src/ghidra/GhidraProvider.ts | 10 ++ src/providers/AnalysisClientCreation.ts | 13 +- 4 files changed, 283 insertions(+), 2 deletions(-) diff --git a/src/application/binary/DeferredAnalysisClient.test.ts b/src/application/binary/DeferredAnalysisClient.test.ts index 587a230ba8..2a94e8445e 100644 --- a/src/application/binary/DeferredAnalysisClient.test.ts +++ b/src/application/binary/DeferredAnalysisClient.test.ts @@ -255,6 +255,163 @@ it("joins a late acquired client's final cleanup before cancellation settles", a ]); }); +it("waits for abandoned creation cleanup before sharing a fresh client", async () => { + const entered = gate(), + loading = gate(), + closeEntered = gate(), + closing = gate(); + let creates = 0; + const client = new DeferredAnalysisClient(async () => { + creates++; + if (creates > 1) return child(); + entered.resolve(); + return loading.promise; + }, identity.id); + const controller = new AbortController(); + const reason = { request: "first caller" }; + const first = client.execute("health", {}, { signal: controller.signal }); + let second: ReturnType | undefined; + let third: ReturnType | undefined; + await withVerifierCleanup(async () => { + await awaitAcknowledgement(entered.promise, first, "client creation"); + controller.abort(reason); + loading.resolve( + child(async () => { + closeEntered.resolve(); + await closing.promise; + return ok(null); + }), + ); + await awaitAcknowledgement(closeEntered.promise, first, "client cleanup"); + second = client.execute("health", {}); + third = client.execute("health", {}); + expect(creates).toBe(1); + closing.resolve(); + const cancelled = await first; + if (cancelled.ok) throw new Error("Expected first caller cancellation"); + expect(cancelled.error.cause).toBe(reason); + expect((await second).ok).toBe(true); + expect((await third).ok).toBe(true); + expect(creates).toBe(2); + }, [ + () => controller.abort(), + () => loading.resolve(child()), + () => closing.resolve(), + async () => { + await Promise.allSettled([first, second, third]); + }, + async () => { + const closed = await client.close(); + if (!closed.ok) throw closed.error; + }, + ]); +}); + +it.each(["caller", "close", "cleanup", "physical"] as const)( + "preserves %s while a new caller waits for abandoned creation", + async (outcome) => { + const entered = gate(), + aborted = gate(), + acquisition = gate(), + release = gate(), + cleanupEntered = gate(); + const firstController = new AbortController(); + const secondController = new AbortController(); + const physical = new ProviderAdapterError( + identity.id, + "create_analysis_client", + { diagnostics: { reason: "physical fixture fault" } }, + ); + let creates = 0; + let denied = outcome === "cleanup"; + const client = new DeferredAnalysisClient(async (signal) => { + creates++; + signal.addEventListener("abort", () => aborted.resolve(), { once: true }); + entered.resolve(); + if (outcome === "physical") { + await release.promise; + throw physical; + } + await acquisition.promise; + return child(async () => { + cleanupEntered.resolve(); + await release.promise; + return denied + ? err( + new ProviderCleanupError(identity.id, ["controlled child"], { + reason: "close refused", + }), + ) + : ok(null); + }); + }, identity.id); + const first = client.execute( + "health", + {}, + { signal: firstController.signal }, + ); + let second: ReturnType | undefined; + let closing: ReturnType | undefined; + await withVerifierCleanup(async () => { + await awaitAcknowledgement(entered.promise, first, "client creation"); + firstController.abort("first caller"); + await awaitAcknowledgement(aborted.promise, first, "creation abort"); + if (outcome !== "physical") { + acquisition.resolve(); + await awaitAcknowledgement( + cleanupEntered.promise, + first, + "client cleanup", + ); + } + second = client.execute( + "analyze_function", + {}, + { + signal: secondController.signal, + }, + ); + if (outcome === "caller") secondController.abort("second caller"); + if (outcome === "close") closing = client.close(); + release.resolve(); + const result = await second; + if (result.ok) throw new Error("Expected interrupted creation"); + if (outcome === "caller") { + expect(result.error).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cause).toBe("second caller"); + } else if (outcome === "close") { + expect(result.error).toMatchObject({ + diagnostics: { reason: "client_closed" }, + }); + expect((await closing)?.ok).toBe(true); + } else if (outcome === "cleanup") { + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupResources).toContain("controlled child"); + } else { + expect(result.error).toMatchObject({ + diagnostics: { reason: "physical fixture fault" }, + }); + expect(result.error.cause).toBe(physical); + } + expect((await first).ok).toBe(false); + expect(creates).toBe(1); + }, [ + () => firstController.abort(), + () => secondController.abort(), + () => acquisition.resolve(), + () => release.resolve(), + async () => { + await Promise.allSettled([first, second, closing]); + }, + async () => { + denied = false; + const closed = await client.close(); + if (!closed.ok) throw closed.error; + }, + ]); + }, +); + it("keeps a producer-authored physical failure primary even when it is the caller's abort reason", async () => { const entered = gate(), finishing = gate(); diff --git a/src/ghidra/GhidraProvider.behavior.test.ts b/src/ghidra/GhidraProvider.behavior.test.ts index e40756c31d..61b43133ec 100644 --- a/src/ghidra/GhidraProvider.behavior.test.ts +++ b/src/ghidra/GhidraProvider.behavior.test.ts @@ -22,6 +22,7 @@ import { type AnalysisCallerLifetime, } from "../domain/analysisErrorBase.js"; import { createDeferred } from "../../tests/fixtures/binarySession.js"; +import { awaitAcknowledgement } from "../../tests/fixtures/lifecycleAcknowledgement.js"; import { ProviderInspectionScope } from "../application/ProviderInspectionScope.js"; import { ok as javaProbeOk, err, ok } from "../domain/result.js"; import type { GhidraJavaObservation } from "./GhidraJavaProbe.js"; @@ -80,7 +81,10 @@ import { GhidraLaunchError, type GhidraHeadlessLauncher, } from "./GhidraLauncher.js"; -import { AnalysisArtifactChangedError } from "../domain/analysisErrorCore.js"; +import { + AnalysisArtifactChangedError, + AnalysisCancelledError, +} from "../domain/analysisErrorCore.js"; import { GHIDRA_PROJECT_PROFILE_MISMATCH } from "./GhidraProjectSnapshot.js"; import type { GhidraInstallationHost } from "./GhidraInstallation.js"; import { @@ -5943,6 +5947,105 @@ it("reuses readiness only inside its active task and provider lifetime", async ( } }); +it.each(["retry", "physical", "cleanup", "close"] as const)( + "handles %s during an abandoned installation inspection", + async (outcome) => { + const entered = createDeferred(); + const aborted = createDeferred(); + const release = createDeferred(); + const base = installationHost(); + let probes = 0; + let denyCleanup = outcome === "cleanup"; + const physical = new AnalysisArtifactChangedError( + "ghidra:installation", + "/selected/java", + "Java changed during inspection", + ); + const ghidra = provider({ + ...base, + probeJava: async (command, environment, options) => { + probes++; + if (probes === 1) { + options.signal?.addEventListener("abort", () => aborted.resolve(), { + once: true, + }); + entered.resolve(); + await release.promise; + if (outcome === "physical") return err(physical); + if (outcome === "cleanup") { + options.resourceOwner?.retain( + { + close: async () => { + if (denyCleanup) throw new Error("close refused"); + }, + }, + "controlled Java descriptor", + ); + return err( + new AnalysisCancelledError("ghidra:installation", { + cause: options.signal?.reason, + cleanup: { + reason: "close refused", + resources: ["controlled Java descriptor"], + }, + }), + ); + } + } + return base.probeJava(command, environment, options); + }, + }); + const controller = new AbortController(); + const first = ghidra.inspectAvailability({ signal: controller.signal }); + void first.catch(() => undefined); + let second: ReturnType | undefined; + let closing: ReturnType | undefined; + try { + await awaitAcknowledgement(entered.promise, first, "Java inspection"); + controller.abort("first inspection"); + await awaitAcknowledgement(aborted.promise, first, "inspection abort"); + second = ghidra.inspectAvailability(); + void second.catch(() => undefined); + await Promise.resolve(); + expect(probes).toBe(1); + if (outcome === "close") closing = ghidra.closeOwnedResources(); + release.resolve(); + if (outcome === "retry") { + await expect(first).rejects.toMatchObject({ + _tag: "AnalysisCancelledError", + }); + expect((await second).status).toBe("available"); + expect(probes).toBe(2); + } else if (outcome === "physical") { + await expect(second).rejects.toMatchObject({ + _tag: "AnalysisArtifactChangedError", + path: "/selected/java", + }); + expect(probes).toBe(1); + } else if (outcome === "cleanup") { + await expect(second).rejects.toMatchObject({ + _tag: "AnalysisCancelledError", + cleanupIncomplete: true, + cleanupResources: ["controlled Java descriptor"], + }); + expect(probes).toBe(1); + } else { + await expect(second).rejects.toMatchObject({ + _tag: "AnalysisCancelledError", + }); + expect((await closing)?.ok).toBe(true); + expect(probes).toBe(1); + } + } finally { + controller.abort(); + release.resolve(); + await Promise.allSettled([first, second, closing]); + denyCleanup = false; + expect((await ghidra.closeOwnedResources()).ok).toBe(true); + } + }, +); + it("refuses a changed installation before creating a target-bound native client", async () => { let release = "12.1.4", nativeCreates = 0; diff --git a/src/ghidra/GhidraProvider.ts b/src/ghidra/GhidraProvider.ts index f5934b2fd5..15d25ef2b4 100644 --- a/src/ghidra/GhidraProvider.ts +++ b/src/ghidra/GhidraProvider.ts @@ -650,6 +650,7 @@ export class GhidraProvider implements AnalysisProviderCandidate { return observed.installation; } let attempt = this.#inspectionAttempt; + const draining = attempt?.signal.aborted === true; if (attempt === undefined) { attempt = new SharedStartupAttempt( (signal) => @@ -686,6 +687,15 @@ export class GhidraProvider implements AnalysisProviderCandidate { throw new AnalysisCancelledError(operation, { cause: callerSignal?.reason, }); + if ( + draining && + !callerSignal?.aborted && + !waited.value.ok && + waited.value.error instanceof AnalysisCancelledError && + !waited.value.error.cleanupIncomplete && + waited.value.error.cause === attempt.signal.reason + ) + return this.#inspectInstallation(callerSignal, operation, scope); if (!waited.value.ok) throw analysisClientFailureForOperation( waited.value.error, diff --git a/src/providers/AnalysisClientCreation.ts b/src/providers/AnalysisClientCreation.ts index 8384999b09..d79d593f7e 100644 --- a/src/providers/AnalysisClientCreation.ts +++ b/src/providers/AnalysisClientCreation.ts @@ -95,6 +95,7 @@ export class AnalysisClientCreation { this.#clientLease = undefined; } let attempt = this.#attempt; + const draining = attempt?.signal.aborted === true; if (attempt === undefined) { this.#failure = undefined; attempt = new SharedStartupAttempt((ownerSignal) => @@ -103,8 +104,9 @@ export class AnalysisClientCreation { const owned = attempt; this.#attempt = owned; void owned.promise.then((result) => { + if (this.#attempt !== owned) return; if (!result.ok) this.#failure = result.error; - if (this.#attempt === owned) this.#attempt = undefined; + this.#attempt = undefined; }); } const waited = await attempt.wait(signal); @@ -112,6 +114,15 @@ export class AnalysisClientCreation { return err( new AnalysisCancelledError(operation, { cause: signal?.reason }), ); + if ( + draining && + !signal?.aborted && + !waited.value.ok && + waited.value.error instanceof AnalysisCancelledError && + !waited.value.error.cleanupIncomplete && + waited.value.error.cause === attempt.signal.reason + ) + return this.acquire(operation, signal); if (!waited.value.ok) return err( analysisClientFailureForOperation(