fix(kuna): keep the run slot of an unproven stop until close proves it (#4052)

A single-function run whose process tree could not be proven stopped
released its run slot immediately, so the next queued run started while
the old tree might still run, and repeated unproven stops bypassed the
bound. The retained run now carries its slot, and close releases it only
after a retried stop is proven.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

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.
 

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<Promise<unknown>>();
-  /** 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?.();
     }
   }
 

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(

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) };
+};

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();
+  }
+});