diff --git a/docs/kuna-provider.md b/docs/kuna-provider.md index d4f9eee85d..b860b4c251 100644 --- a/docs/kuna-provider.md +++ b/docs/kuna-provider.md @@ -44,7 +44,7 @@ The first analysis request in a session looks for this target's export in the ca - **Cold cache:** - REA starts `kuna decompile-project` in the background on a private copy of the target. - Meanwhile, pseudocode comes from one `kuna decompile --addr` run, and listings from `kuna functions --json`. - - Requests for the same function share one run; it stops when its last waiting request is cancelled. Each run analyzes the whole image again in its own process, so one REA process runs at most as many at once as the host has cores beyond `REA_KUNA_JOBS` (at least one). Further requests wait in order, and a waiting request is answered from the export if it finishes first. + - Requests for the same function share one run; it stops when its last waiting request is cancelled. Each run analyzes the whole image again in its own process, so one REA process runs at most as many at once as the host has cores beyond `REA_KUNA_JOBS` (at least one). Further requests wait in order, and a waiting request is answered from the export if it finishes first. A run whose stop REA cannot prove keeps its slot until `close_binary` proves it stopped. - Assembly and data names wait for the export; progress notifications report the wait every ten seconds. - A single-function answer says so in its limitations. It lacks the export's whole-program prototypes, so a later request can return different text for the same function. diff --git a/src/kuna/KunaSessionClient.ts b/src/kuna/KunaSessionClient.ts index d9f53d2e95..6faaecfd1a 100644 --- a/src/kuna/KunaSessionClient.ts +++ b/src/kuna/KunaSessionClient.ts @@ -173,10 +173,14 @@ export class KunaSessionClient implements AnalysisClient { readonly #session = new AbortController(); /** Requests and shared readers retain their resources until their work settles. */ readonly #active = new Set>(); - /** Single-function runs that could not be proven stopped, with their retry owners. */ + /** + * Kuna runs that could not be proven stopped, with their retry owners and, + * for single-function runs, the run slot their process tree still holds. + */ readonly #retained: { readonly error: ProviderCleanupError; owner: ProviderProcessSupervisor | undefined; + readonly slot?: KunaRunRelease; }[] = []; /** Private roots whose removal failed during setup or close. */ readonly #leakedRoots: PrivateRuntimeRoot[] = []; @@ -357,8 +361,10 @@ export class KunaSessionClient implements AnalysisClient { } for (const retained of [...this.#retained]) { const stopped = await retained.owner?.stop(); - if (stopped !== undefined && stopped.status !== "incomplete") + if (stopped !== undefined && stopped.status !== "incomplete") { this.#retained.splice(this.#retained.indexOf(retained), 1); + retained.slot?.(); + } } failures.push(...this.#retained.map(({ error }) => error)); const workspace = await this.#workspace; @@ -787,6 +793,7 @@ export class KunaSessionClient implements AnalysisClient { if (admitted === EXPORT_READY) return admitted; if (admitted === undefined) return err(new AnalysisCancelledError(operation)); + let slot: KunaRunRelease | undefined = admitted; try { const run = await runKunaCommand({ installation, @@ -800,13 +807,15 @@ export class KunaSessionClient implements AnalysisClient { launcher: this.#launcher, }); if (run.kind === "retained") { - this.#retained.push({ error: run.error, owner: run.owner }); + // The tree may still run, so it keeps its slot until close proves it stopped. + this.#retained.push({ error: run.error, owner: run.owner, slot }); + slot = undefined; return err(run.error); } if (run.kind === "failed") return err(run.error); return ok(run.snapshot.stdout.text.replace(/\n+$/u, "")); } finally { - admitted(); + slot?.(); } } diff --git a/tests/boundary/providers/kuna/kunaProvider.test.ts b/tests/boundary/providers/kuna/kunaProvider.test.ts index c4d87290ff..1613929cf0 100644 --- a/tests/boundary/providers/kuna/kunaProvider.test.ts +++ b/tests/boundary/providers/kuna/kunaProvider.test.ts @@ -27,7 +27,13 @@ import { writeFakeKuna, } from "./fakeKuna.js"; -import { session, run, exportsOf, FUNCTIONS } from "./kunaSessionFixture.js"; +import { + session, + run, + exportsOf, + FUNCTIONS, + unprovenCleanupLauncher, +} from "./kunaSessionFixture.js"; const it = test.skipIf(process.platform === "win32"); @@ -547,28 +553,6 @@ it("refuses another document, an unsupported operation and a replaced executable await replaced.close(); }); -/** - * Launch for real, but have cleanup of the selected subcommand report an - * unproven stop until `release` is called, as a lost ownership proof would. - */ -const unprovenCleanupLauncher = (subcommand: string) => { - let proven = false; - const launcher: KunaProcessLauncher = async (options) => { - const spawned = await spawnOwnedProviderProcess(options); - if (options.arguments[0] !== subcommand) return spawned; - return { - ...spawned, - cleanup: async () => { - const cleaned = await cleanupOwnedProcessGroup(spawned.ownership); - return proven - ? cleaned - : { cleaned: false, reason: `${subcommand} stop unproven` }; - }, - }; - }; - return { launcher, release: () => (proven = true) }; -}; - it("retains an active request's workspace when close cannot prove its stop", async () => { const unproven = unprovenCleanupLauncher("decompile"); const { kuna, open } = await session( diff --git a/tests/boundary/providers/kuna/kunaSessionFixture.ts b/tests/boundary/providers/kuna/kunaSessionFixture.ts index c498c5f088..dc53b29a0e 100644 --- a/tests/boundary/providers/kuna/kunaSessionFixture.ts +++ b/tests/boundary/providers/kuna/kunaSessionFixture.ts @@ -6,6 +6,8 @@ import type { BinaryTarget } from "../../../../src/domain/binaryTargetTypes.js"; import type { JsonValue } from "../../../../src/domain/jsonValue.js"; import type { KunaProcessLauncher } from "../../../../src/kuna/KunaCommand.js"; import { KunaProvider } from "../../../../src/kuna/KunaProvider.js"; +import { cleanupOwnedProcessGroup } from "../../../../src/process/ProcessOwnership.js"; +import { spawnOwnedProviderProcess } from "../../../../src/process/ProviderProcess.js"; import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; import { fakeKunaCalls, @@ -78,3 +80,25 @@ export const exportsOf = async (kuna: string) => (await fakeKunaCalls(kuna)).filter( ({ argv }) => argv[0] === "decompile-project", ); + +/** + * Launch for real, but have cleanup of the selected subcommand report an + * unproven stop until `release` is called, as a lost ownership proof would. + */ +export const unprovenCleanupLauncher = (subcommand: string) => { + let proven = false; + const launcher: KunaProcessLauncher = async (options) => { + const spawned = await spawnOwnedProviderProcess(options); + if (options.arguments[0] !== subcommand) return spawned; + return { + ...spawned, + cleanup: async () => { + const cleaned = await cleanupOwnedProcessGroup(spawned.ownership); + return proven + ? cleaned + : { cleaned: false, reason: `${subcommand} stop unproven` }; + }, + }; + }; + return { launcher, release: () => (proven = true) }; +}; diff --git a/tests/boundary/providers/kuna/kunaSingleFunctionRuns.test.ts b/tests/boundary/providers/kuna/kunaSingleFunctionRuns.test.ts index ddedc4873d..33f02dd721 100644 --- a/tests/boundary/providers/kuna/kunaSingleFunctionRuns.test.ts +++ b/tests/boundary/providers/kuna/kunaSingleFunctionRuns.test.ts @@ -1,7 +1,16 @@ import { expect, it as test } from "vitest"; -import { fakeKunaCalls, fakeKunaExits } from "./fakeKuna.js"; -import { run, session } from "./kunaSessionFixture.js"; +import { + fakeKunaCalls, + fakeKunaExits, + setFakeKunaBehavior, +} from "./fakeKuna.js"; +import { + FUNCTIONS, + run, + session, + unprovenCleanupLauncher, +} from "./kunaSessionFixture.js"; const it = test.skipIf(process.platform === "win32"); @@ -127,3 +136,53 @@ it("answers a queued request from the export once it finishes first", async () = expect(await client.close()).toEqual({ ok: true, value: null }); } }); + +it("keeps the slot of a run whose stop is unproven until close proves it", async () => { + const unproven = unprovenCleanupLauncher("decompile"); + const { kuna, open } = await session( + "rea-kuna-single-retained-slot-", + { project: "hang", decompile: "hang" }, + { REA_KUNA_TIMEOUT_MS: "300" }, + unproven.launcher, + 1, + ); + const client = open(); + const waiter = new AbortController(); + try { + expect( + await client.execute("procedure_pseudo_code", { procedure: "main" }), + ).toMatchObject({ ok: false, error: { cleanupIncomplete: true } }); + // The timed-out tree may still run, so the next run must not start. + const queued = client.execute( + "procedure_pseudo_code", + { procedure: "parse" }, + { signal: waiter.signal }, + ); + await new Promise((resolve) => setTimeout(resolve, 300)); + expect(await decompilesOf(kuna)).toHaveLength(1); + waiter.abort(); + expect(await queued).toMatchObject({ + ok: false, + error: { _tag: "AnalysisCancelledError" }, + }); + expect((await client.close()).ok).toBe(false); + unproven.release(); + expect(await client.close()).toEqual({ ok: true, value: null }); + + // The proven stop returns the slot to the provider. + await setFakeKunaBehavior(kuna, { functions: FUNCTIONS, project: "hang" }); + const next = open(); + try { + expect( + (await run(next, "procedure_pseudo_code", { procedure: "parse" })) + .rawResult, + ).toMatchObject({ source: { kind: "single-function" } }); + } finally { + expect(await next.close()).toEqual({ ok: true, value: null }); + } + } finally { + waiter.abort(); + unproven.release(); + await client.close(); + } +});