From b6e6d7cd1a64c0f4978f01e50d8d2fd04f8bbda3 Mon Sep 17 00:00:00 2001 From: RODRIGO DE SOUZA FARIA <45077848+alucardigo@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:38:48 -0300 Subject: [PATCH] fix(ida): ignore released worker rows when verifying close (#4234) * fix(ida): ignore released worker rows when verifying close Upstream idb_list can observe the owning worker mid-teardown and return a row whose identity is already cleared - empty session_id, owned false, is_active false - while input_path still names the private copy. The post-close check matched that row through its private-input arm and threw "IDA worker remains registered after close.", failing a run whose analysis had completed and whose idb_close was already acknowledged, and retaining the private workspace. Such a row establishes no live session, so it no longer blocks release. A genuine peer on the same input keeps a session ID and is still reported, and the session-ID arm is untouched. Measured against IDA Pro 9.2 on Windows: 11 of 12 stock runs failed this way; 6 of 6 passed with this change and retained no workspace. Refs #3056 Co-Authored-By: Claude Opus 5 * fix(ida): suppress released rows only for worker backends The blanked-identity exception reached a gui row as well, and a gui session belongs to an IDA instance REA does not control, so a same-input gui binding could stop blocking release. The observed teardown shape covers workers only, so require backend "worker" before suppressing. Adds a regression test keeping a blank-identity gui row on the same private input reported as a blocker, and moves the three blanked-identity tests into their own describe to stay within the per-block line limit. Refs #3056 Co-Authored-By: Claude Opus 5 * refactor(ida): state the released-row predicate as equality `owned` and `is_active` are strict booleans in databaseListSchema, so `!== true` suggested a third possible value that cannot occur. Say what the row actually asserts. No behaviour change; the blanked-identity tests cover both directions. Refs #3056 Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- src/ida/IdaSessionClient.test.ts | 94 ++++++++++++++++++++++++++++++++ src/ida/IdaSessionClient.ts | 34 ++++++++++-- 2 files changed, 123 insertions(+), 5 deletions(-) diff --git a/src/ida/IdaSessionClient.test.ts b/src/ida/IdaSessionClient.test.ts index 2b0e6fb835..0de23b309e 100644 --- a/src/ida/IdaSessionClient.test.ts +++ b/src/ida/IdaSessionClient.test.ts @@ -1140,6 +1140,100 @@ describe("headless IDA session ownership and cleanup", () => { }); }); +describe("headless IDA blanked-identity inventory rows", () => { + it("releases cleanly when the closed worker lingers in inventory without an identity", async () => { + const { producer, client, root } = await fixture("headless"); + expect((await client.execute("health", {})).ok).toBe(true); + let released = false; + producer.beforeCall = async (name) => { + if (name === "idb_close") released = true; + }; + // A worker observed mid-teardown: identity cleared, private input retained. + producer.overrideCall = (name) => + name === "idb_list" && released + ? { + sessions: [ + { + session_id: "", + input_path: producer.inputPath, + owned: false, + backend: "worker", + is_active: false, + }, + ], + count: 1, + } + : undefined; + + expect((await client.close()).ok).toBe(true); + expect(await readdir(root)).toEqual(["target.elf"]); + }); + + it("still reports a same-input gui row whose identity is blank", async () => { + const { producer, client, root } = await fixture("headless"); + expect((await client.execute("health", {})).ok).toBe(true); + let released = false; + producer.beforeCall = async (name) => { + if (name === "idb_close") released = true; + }; + // A gui binding belongs to an IDA instance REA does not control, so the + // blanked-identity exception must not reach it. + producer.overrideCall = (name) => + name === "idb_list" && released + ? { + sessions: [ + { + session_id: "", + input_path: producer.inputPath, + owned: false, + backend: "gui", + is_active: false, + }, + ], + count: 1, + } + : undefined; + + const result = await client.close(); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.error.cleanupIncomplete).toBe(true); + expect( + (await readdir(root)).some((name) => name.startsWith("rea-ida-")), + ).toBe(true); + }); + + it("still reports a same-input peer that keeps a session identity", async () => { + const { producer, client, root } = await fixture("headless"); + expect((await client.execute("health", {})).ok).toBe(true); + let released = false; + producer.beforeCall = async (name) => { + if (name === "idb_close") released = true; + }; + producer.overrideCall = (name) => + name === "idb_list" && released + ? { + sessions: [ + { + session_id: "unrelated-live-peer", + input_path: producer.inputPath, + owned: false, + backend: "worker", + is_active: false, + }, + ], + count: 1, + } + : undefined; + + const result = await client.close(); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.error.cleanupIncomplete).toBe(true); + expect( + (await readdir(root)).some((name) => name.startsWith("rea-ida-")), + ).toBe(true); + }); +}); + describe("headless IDA reaped worker cleanup", () => { it("recovers a reaped worker only after inventory confirms its session and private input are absent", async () => { const { producer, client, root } = await fixture("headless"); diff --git a/src/ida/IdaSessionClient.ts b/src/ida/IdaSessionClient.ts index 5c00179906..06a418e763 100644 --- a/src/ida/IdaSessionClient.ts +++ b/src/ida/IdaSessionClient.ts @@ -152,6 +152,29 @@ class IdaCleanupError extends ProviderAdapterError { } } +/** + * A worker that is still tearing down can remain in inventory with its identity + * already cleared: an empty `session_id`, `owned: false` and `is_active: false`, + * while `input_path` still names the private copy. Such a row establishes no + * live session, so the private-input arm must not report it as a blocker. The + * owning worker is matched by its own session ID separately, and a genuine peer + * on the same input keeps a session ID, so neither case is weakened. + * + * Limited to `backend: "worker"`: a gui row belongs to an IDA instance REA does + * not control, and the observed teardown shape covers workers only, so a gui + * row keeps blocking release whatever its flags say. + */ +const isReleasedWorkerRow = (entry: { + readonly session_id: string; + readonly owned: boolean; + readonly is_active: boolean; + readonly backend: "worker" | "gui"; +}): boolean => + entry.backend === "worker" && + entry.session_id === "" && + entry.owned === false && + entry.is_active === false; + /** Serial read-only session whose lifecycle authority excludes an attached GUI. */ export class IdaSessionClient implements AnalysisClient { #tail: Promise = Promise.resolve(); @@ -873,11 +896,12 @@ export class IdaSessionClient implements AnalysisClient { ); const remaining = inventory.sessions.filter( (entry) => - entry.session_id === this.#database || - sameIdaHostPath( - entry.input_path, - this.#workspace?.inputPath ?? "", - ), + !isReleasedWorkerRow(entry) && + (entry.session_id === this.#database || + sameIdaHostPath( + entry.input_path, + this.#workspace?.inputPath ?? "", + )), ); if (this.#recordBlockingSessions(remaining)) throw new AnalysisProtocolError(