mirror of
https://github.com/morluto/rea
synced 2026-10-28 23:11:11 -04:00
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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
parent
6714fb1d02
commit
b6e6d7cd1a
@@ -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");
|
||||
|
||||
@@ -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<void> = 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(
|
||||
|
||||
Reference in New Issue
Block a user