From d32d9f29020e54b684dcf1a0f39c5b4d9e8637ce Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Sun, 25 Oct 2026 05:21:26 -0700 Subject: [PATCH] fix(lldb): keep the bridge's no-target proof through interruption (#7111) When cancellation or the deadline interrupted LLDB after it had published its bridge result, the tracer never read that result, so an explicit `target_process_created: false` (or `target_process_exit_observed: true`) was lost. The run kept a `native-target:unknown` marker that no cleanup retry could release, wedging the tracer for every later trace. Read the published result once LLDB's process group is stopped, even for an interrupted run, and apply the same target-gone proof in cleanup retry. Cancellation or timeout stays the primary outcome; a created target, a truncated result, or a missing PID alone stays unresolved. Fixes #5385 Co-authored-by: Claude Opus 5.5 --- src/native/LldbCallTracer.ts | 74 ++++-- .../native/lldbCallTracerFailures.test.ts | 226 ++++++++++++++++++ 2 files changed, 274 insertions(+), 26 deletions(-) diff --git a/src/native/LldbCallTracer.ts b/src/native/LldbCallTracer.ts index c9b508f421..c2c5d510ce 100644 --- a/src/native/LldbCallTracer.ts +++ b/src/native/LldbCallTracer.ts @@ -244,6 +244,7 @@ export class LldbCallTracer implements NativeCallTracer { files: new LldbReadbackFiles(), supervisor: undefined, pidPath: paths.pid, + resultPath: paths.result, targetIdentity: undefined, targetPid: undefined, processStopped: true, @@ -433,16 +434,29 @@ export class LldbCallTracer implements NativeCallTracer { } if (owner.processStopped && !owner.targetStopped) { owner.targetPid = await owner.files.readTargetPid(owner.pidPath); - try { - const targetTermination = await ensureTerminated( + // A stopped LLDB may have published its result before interruption. + const published = await readTracerOutput(owner.resultPath, owner.files); + if ( + bridgeProvesTargetGone( + published.ok ? published.value : undefined, owner.targetPid, - owner.targetIdentity, - ); - if (targetTermination !== "terminated") - failures.push(new Error(targetTerminationReason(targetTermination))); - else owner.targetStopped = true; - } catch (cause: unknown) { - failures.push(cause); + ) + ) + owner.targetStopped = true; + else { + try { + const targetTermination = await ensureTerminated( + owner.targetPid, + owner.targetIdentity, + ); + if (targetTermination !== "terminated") + failures.push( + new Error(targetTerminationReason(targetTermination)), + ); + else owner.targetStopped = true; + } catch (cause: unknown) { + failures.push(cause); + } } } if (failures.length === 0) { @@ -646,29 +660,22 @@ export class LldbCallTracer implements NativeCallTracer { retained.processStopped = exited.cleanupOwner === undefined && exited.cleanupFailure === undefined; retained.targetStopped = !exited.targetLaunched; + // The bridge publishes its result once, at the end of rea_trace. Once LLDB + // stopped, an interrupted run still owns that result's target facts, while + // only an exited LLDB selects it as the primary outcome. const readback = - exited.kind === "exited" + exited.kind === "exited" || + (exited.targetLaunched && retained.processStopped) ? await readTracerOutput(paths.result, retained.files) : undefined; - const output = readback?.ok ? readback.value : undefined; - // An explicit bridge observation can prove no process was created or that - // the created process exited. A missing PID alone remains ambiguous. - const bridgeObservedExit = - output !== undefined && - output.status !== "traced" && - output.target_process_exit_observed === true; + const published = readback?.ok ? readback.value : undefined; + const output = exited.kind === "exited" ? published : undefined; retained.targetPid = await retained.files.readTargetPid(paths.pid); - let targetTermination: TargetTermination = - exited.targetLaunched && !bridgeObservedExit + const targetTermination: TargetTermination = + exited.targetLaunched && + !bridgeProvesTargetGone(published, retained.targetPid) ? await ensureTerminated(retained.targetPid, exited.targetIdentity) : "terminated"; - if ( - output !== undefined && - output.status !== "traced" && - output.target_process_created === false && - retained.targetPid === undefined - ) - targetTermination = "terminated"; retained.targetStopped = targetTermination === "terminated"; if ( retained.processStopped && @@ -1136,6 +1143,20 @@ const readTracerOutput = async ( return output.success ? ok(output.data) : err(output.error); }; +/** + * An explicit bridge observation can prove no process was created (with no + * published PID) or that the created process exited. A missing PID alone, an + * unreadable result, or a traced result remains ambiguous. + */ +const bridgeProvesTargetGone = ( + output: z.infer | undefined, + targetPid: number | undefined, +): boolean => + output !== undefined && + output.status !== "traced" && + (output.target_process_exit_observed === true || + (output.target_process_created === false && targetPid === undefined)); + const capturedOutput = async ( path: string, observedBytes: number, @@ -1184,6 +1205,7 @@ interface RetainedLldbResources { readonly rootLease: AnalysisCleanupLease; readonly files: LldbReadbackFiles; readonly pidPath: string; + readonly resultPath: string; supervisor: ProviderProcessSupervisor | undefined; targetIdentity: string | undefined; targetPid: number | undefined; diff --git a/tests/boundary/providers/native/lldbCallTracerFailures.test.ts b/tests/boundary/providers/native/lldbCallTracerFailures.test.ts index 8cd5999666..1ee4cdda35 100644 --- a/tests/boundary/providers/native/lldbCallTracerFailures.test.ts +++ b/tests/boundary/providers/native/lldbCallTracerFailures.test.ts @@ -855,6 +855,232 @@ describe("LLDB process exit while output is still open", () => { ); }); +/** + * A fake LLDB that publishes its bridge result and exits 0, as `lldb --batch` + * does after rea_trace, while an inherited child keeps its output open. + */ +const publishedResultLldb = async (result: string) => { + const fixtureRoot = await mkdtemp(join(tmpdir(), "rea-lldb-published-")); + const fakeLldb = join(fixtureRoot, "fake-lldb"); + await writeFile( + fakeLldb, + `#!${process.execPath}\nconst fs = require("node:fs");\nconst { spawn } = require("node:child_process");\nconst command = process.argv.at(-1);\nif (!command?.startsWith("rea_trace ")) process.exit(91);\nconst config = JSON.parse(fs.readFileSync(command.slice("rea_trace ".length), "utf8"));\nconst child = spawn(process.execPath, ["-e", "setTimeout(() => {}, 60000)"], { stdio: ["ignore", "inherit", "inherit"] });\nchild.once("error", () => process.exit(92));\nchild.once("spawn", () => { fs.writeFileSync(config.result_path, ${JSON.stringify(result)}); process.exit(0); });\n`, + { mode: 0o700 }, + ); + return { fixtureRoot, fakeLldb }; +}; + +const publishedResults = { + "no target created": JSON.stringify({ + status: "target-error", + error: "fixture target creation failure", + target_process_created: false, + }), + "target exit observed": JSON.stringify({ + status: "launch-error", + error: "fixture launch failure after exit", + target_process_created: true, + target_process_exit_observed: true, + }), + "target created": JSON.stringify({ + status: "target-integrity-error", + error: "fixture post-launch integrity failure", + target_process_created: true, + }), + "truncated result": `{"status":"target-error","error":null,"target_process_created":false`, +} as const; +const provesTargetGone = (published: keyof typeof publishedResults) => + published === "no target created" || published === "target exit observed"; + +/** Interrupt once, after the LLDB leader exited while inherited output is open. */ +const interruptAfterLeaderExit = (interrupt: () => void) => { + let interrupted = false; + const originalWait = ProviderProcessSupervisor.prototype.waitForOutputClose; + const spy = vi + .spyOn(ProviderProcessSupervisor.prototype, "waitForOutputClose") + .mockImplementation(async function ( + this: ProviderProcessSupervisor, + timeoutMs?: number, + signal?: AbortSignal, + ): Promise { + const closed = await originalWait.call(this, timeoutMs, signal); + if (!closed && !interrupted && this.observedExit()?.code === 0) { + interrupted = true; + interrupt(); + } + return closed; + }); + return { + get interrupted() { + return interrupted; + }, + restore: () => spy.mockRestore(), + }; +}; + +describe("LLDB target proof published before interruption", () => { + for (const interruption of [ + "caller cancellation", + "deadline timeout", + ] as const) + for (const published of Object.keys(publishedResults) as Array< + keyof typeof publishedResults + >) + it.skipIf(process.platform === "win32")( + `keeps the bridge's ${published} fact when ${interruption} arrives after LLDB exit and before result readback`, + async () => { + const { fixtureRoot, fakeLldb } = await publishedResultLldb( + publishedResults[published], + ); + const controller = new AbortController(); + const leader = interruptAfterLeaderExit(() => { + if (interruption === "caller cancellation") + controller.abort(new Error("abort before result readback")); + }); + const deadlineSpy = + interruption === "deadline timeout" + ? vi + .spyOn( + ProviderStartupDeadline.prototype, + "interruption", + "get", + ) + .mockImplementation(() => + leader.interrupted ? "timeout" : undefined, + ) + : undefined; + const tracer = new LldbCallTracer({}, undefined, async () => + ok({ path: fakeLldb, sha256: "c".repeat(64) }), + ); + let retainedRoot: string | undefined; + try { + const result = await tracer.trace(request, controller.signal); + + expect(leader.interrupted).toBe(true); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected an interrupted trace"); + expect(result.error).toBeInstanceOf( + interruption === "caller cancellation" + ? AnalysisCancelledError + : AnalysisTimeoutError, + ); + expect(result.error.partialObservation).toMatchObject({ + coverage: { + reason: + interruption === "caller cancellation" + ? "cancelled" + : "timeout", + }, + }); + if (provesTargetGone(published)) { + expect(result.error.cleanupIncomplete).toBe(false); + expect(result.error.cleanupResources).toEqual([]); + expect((await tracer.close()).ok).toBe(true); + return; + } + // Without an explicit producer proof, a missing PID stays ambiguous. + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupResources).toContain( + "native-target:unknown", + ); + expect(result.error.cleanupResources).not.toContain( + "lldb-process-group", + ); + retainedRoot = result.error.cleanupResources.find((resource) => + resource.includes("rea-lldb-"), + ); + const close = await tracer.close(); + expect(close.ok).toBe(false); + if (!close.ok) + expect(close.error.cleanupResources).toContain( + "native-target:unknown", + ); + } finally { + leader.restore(); + deadlineSpy?.mockRestore(); + if (retainedRoot !== undefined) + await rm(retainedRoot, { recursive: true, force: true }); + await rm(fixtureRoot, { recursive: true, force: true }); + } + }, + ); +}); + +describe("LLDB target proof read by cleanup retry", () => { + for (const published of ["no target created", "target created"] as const) + it.skipIf(process.platform === "win32")( + `reads the published ${published} fact when cleanup retry releases interrupted LLDB`, + async () => { + const { fixtureRoot, fakeLldb } = await publishedResultLldb( + publishedResults[published], + ); + const controller = new AbortController(); + const leader = interruptAfterLeaderExit(() => + controller.abort(new Error("abort before result readback")), + ); + const originalStop = ProviderProcessSupervisor.prototype.stop; + let stops = 0; + const stopSpy = vi + .spyOn(ProviderProcessSupervisor.prototype, "stop") + .mockImplementation(async function ( + this: ProviderProcessSupervisor, + options?: ProviderProcessStopOptions, + ): Promise { + stops += 1; + return stops === 1 + ? { status: "incomplete", reason: "fixture held LLDB cleanup" } + : originalStop.call(this, options); + }); + const tracer = new LldbCallTracer({}, undefined, async () => + ok({ path: fakeLldb, sha256: "c".repeat(64) }), + ); + let retainedRoot: string | undefined; + try { + const result = await tracer.trace(request, controller.signal); + + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected a cancelled trace"); + expect(result.error).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupResources).toEqual( + expect.arrayContaining([ + "lldb-process-group", + "native-target:unknown", + ]), + ); + retainedRoot = result.error.cleanupResources.find((resource) => + resource.includes("rea-lldb-"), + ); + expect(retainedRoot).toBeDefined(); + + // The first pass could not read the result while LLDB was unstopped. + const close = await tracer.close(); + expect(stops).toBe(2); + if (published === "no target created") { + expect(close.ok).toBe(true); + expect(result.error.cleanupIncomplete).toBe(false); + await expect(access(retainedRoot ?? "")).rejects.toThrow(); + return; + } + expect(close.ok).toBe(false); + if (close.ok) return; + expect(close.error.cleanupResources).toContain( + "native-target:unknown", + ); + expect(close.error.cleanupResources).not.toContain( + "lldb-process-group", + ); + } finally { + leader.restore(); + stopSpy.mockRestore(); + if (retainedRoot !== undefined) + await rm(retainedRoot, { recursive: true, force: true }); + await rm(fixtureRoot, { recursive: true, force: true }); + } + }, + ); +}); + const completedOutputCloseFixture = async () => { const fixtureRoot = await mkdtemp( join(tmpdir(), "rea-lldb-complete-output-"),