fix(pwntools): retain exact cleanup owners through finalization

Give retained processes and runtime roots separate cleanup leases. Bind
composed failures to their exact dependencies so process retirement removes
only process labels and root deletion resolves the remaining markers.
Retain local evidence while descriptor cleanup is unresolved.

Preserve completed decoded facts and the original cancellation reason when
final cleanup itself fails, then expose that primary after successful retry.
Remove the superseded root-error composition branch after descriptor gating.

Validation: 68 focused tests across nine files, including real retained file
handles, late process handshakes, failed final cleanup, mixed child outcomes
and cleanup retry. Static, docs (five checks) and discovery (1577 files) pass.
The producer qualification scope is controlled Node/file fixtures with the
real macOS inspector and a simulated availability gate, not native pwntools
or GDB. Task tokens/CPU/process-family memory are unmeasured; no speed claim.

Fixes #4945
This commit is contained in:
morluto committed 2026-10-18 10:51:51 +08:00
1 parent 08fbeb8bda
commit 1f23e977b5
4 files changed
+261 -49

No files matched your search

+72 -39
View File
@@ -1,3 +1,7 @@
import {
AnalysisCleanupLease,
AnalysisCleanupLeaseGroup,
} from "../../artifacts/AnalysisCleanupLease.js";
import {
bindArtifactFileCleanup,
artifactFileCancellationCause,
@@ -113,7 +117,13 @@ export class PwntoolsDecoder<
#pendingCleanup:
| {
root: Pick<PrivateRuntimeRoot, "path" | "close"> | undefined;
process: ProviderProcessSupervisor | undefined;
rootLease: AnalysisCleanupLease;
process:
| {
readonly supervisor: ProviderProcessSupervisor;
readonly lease: AnalysisCleanupLease;
}
| undefined;
failure: AnalysisError | undefined;
diagnostics: AnalysisCapturedOutput | undefined;
observation: AnalysisPartialObservation | undefined;
@@ -290,6 +300,7 @@ export class PwntoolsDecoder<
root = await this.createRuntime();
this.#pendingCleanup = {
root,
rootLease: AnalysisCleanupLease.forCloseOwner(root, root.path),
process: undefined,
failure: undefined,
diagnostics: undefined,
@@ -487,25 +498,27 @@ export class PwntoolsDecoder<
: result.error.capturedOutput;
}
const cleanup = await this.#completeCleanup(() => this.#retire());
if (cleanup !== undefined)
return err(
result.ok
? cleanup
: analysisErrorWithCleanupFailure(result.error, cleanup, OPERATION),
if (result.ok && options?.signal?.aborted)
result = err(
new AnalysisCancelledError(OPERATION, {
cause: options.signal.reason,
partialObservation: result.value,
capturedOutput: result.value.diagnostics,
}),
);
return result.ok && options?.signal?.aborted
? err(
new AnalysisCancelledError(OPERATION, {
cause: options.signal.reason,
partialObservation: result.value,
capturedOutput: {
stdout: result.value.diagnostics.stdout,
stderr: result.value.diagnostics.stderr,
truncated: result.value.diagnostics.truncated,
},
}),
)
: result;
if (cleanup !== undefined) {
const failure = result.ok
? cleanup
: analysisErrorWithCleanupFailure(
result.error,
cleanup,
OPERATION,
).bindCleanupDependencies([result.error, cleanup]);
if (this.#pendingCleanup !== undefined)
this.#pendingCleanup.failure = failure;
return err(failure);
}
return result;
}
async #runCommand(
@@ -537,8 +550,21 @@ export class PwntoolsDecoder<
? {}
: { signal: options.signal }),
...(this.launcher === undefined ? {} : { launcher: this.launcher }),
retainCleanup: (owner) => {
pending.process = owner;
retainCleanup: (supervisor) => {
const ownership = supervisor.launch.ownership;
pending.process = {
supervisor,
lease: new AnalysisCleanupLease(
async () => {
const stopped = await supervisor.stop();
if (stopped.status === "incomplete")
throw new Error(stopped.reason, { cause: stopped });
},
ownership === undefined
? ["owned-decoder"]
: ownedCommandResources(ownership),
),
};
},
},
),
@@ -555,7 +581,7 @@ export class PwntoolsDecoder<
primary,
cleanup,
this.profile.operation,
);
).bindCleanupDependencies([primary, cleanup]);
}
if (!outcome.ok) throw outcome.error;
return outcome.value;
@@ -590,6 +616,7 @@ export class PwntoolsDecoder<
{ reason: observation.reason },
{ operation: this.profile.operation, cause },
);
bindArtifactFileCleanup(cause, resourceFailure);
failure =
failure === undefined
? resourceFailure
@@ -597,7 +624,7 @@ export class PwntoolsDecoder<
failure,
resourceFailure,
this.profile.operation,
);
).bindCleanupDependencies([failure, resourceFailure]);
}
if (failure !== undefined) throw failure;
})
@@ -626,10 +653,12 @@ export class PwntoolsDecoder<
async #retireProcess(): Promise<ProviderCleanupError | undefined> {
const pending = this.#pendingCleanup;
if (pending?.process === undefined) return undefined;
const stopped = await pending.process.stop();
const ownership = pending.process.launch.ownership;
if (stopped.status === "incomplete")
return new ProviderCleanupError(
const process = pending.process;
const ownership = process.supervisor.launch.ownership;
try {
await process.lease.release();
} catch (cause: unknown) {
const failure = new ProviderCleanupError(
this.identity.id,
ownership === undefined
? [
@@ -638,10 +667,10 @@ export class PwntoolsDecoder<
]
: ownedCommandResources(ownership, pending.root?.path),
{
reason: stopped.reason,
reason: cause instanceof Error ? cause.message : String(cause),
captured_output: {
...(pending.diagnostics ??
capturedPwntoolsOutput(pending.process.snapshot())),
capturedPwntoolsOutput(process.supervisor.snapshot())),
},
previous_error:
pending.failure === undefined
@@ -650,11 +679,18 @@ export class PwntoolsDecoder<
},
{
operation: this.profile.operation,
cause,
...(pending.observation === undefined
? {}
: { partialObservation: pending.observation }),
},
);
new AnalysisCleanupLeaseGroup(failure, [
process.lease,
pending.rootLease,
]);
return failure;
}
pending.process = undefined;
return undefined;
}
@@ -681,6 +717,7 @@ export class PwntoolsDecoder<
},
);
}
if (failure !== undefined) bindArtifactFileCleanup(failure.cause, failure);
const processFailure = await this.#retireProcess();
if (processFailure !== undefined) {
return failure === undefined
@@ -689,13 +726,15 @@ export class PwntoolsDecoder<
failure,
processFailure,
this.profile.operation,
);
).bindCleanupDependencies([failure, processFailure]);
}
// Retained descriptors still own local evidence under this root.
if (failure !== undefined) return failure;
const pending = this.#pendingCleanup;
if (pending?.root !== undefined) {
const root = pending.root;
try {
await root.close();
await pending.rootLease.release();
pending.root = undefined;
} catch (cause: unknown) {
const rootFailure = new ProviderCleanupError(
@@ -719,14 +758,8 @@ export class PwntoolsDecoder<
: { partialObservation: pending.observation }),
},
);
failure =
failure === undefined
? rootFailure
: analysisErrorWithCleanupFailure(
failure,
rootFailure,
this.profile.operation,
);
new AnalysisCleanupLeaseGroup(rootFailure, [pending.rootLease]);
failure = rootFailure;
}
}
if (!this.#resources.hasPending && pending?.root === undefined)
+26 -3
View File
@@ -59,6 +59,7 @@ it.runIf(supportedHost)(
let pid: number | undefined;
let launches = 0;
let cleanupAllowed = false;
let rootRemovalAllowed = false;
let releaseFixture: (() => Promise<void>) | undefined;
const provider = new PwntoolsLayoutProvider(
{ REA_PWNTOOLS_PYTHON: process.execPath },
@@ -85,11 +86,24 @@ it.runIf(supportedHost)(
}),
};
},
() =>
PrivateRuntimeRoot.create({ parent: workspace.root, prefix: "owned-" }),
async () => {
const root = await PrivateRuntimeRoot.create({
parent: workspace.root,
prefix: "owned-",
});
return {
path: root.path,
close: async () => {
if (!rootRemovalAllowed)
throw new Error("root removal refused after process retirement");
await root.close();
},
};
},
);
onTestFinished(async () => {
cleanupAllowed = true;
rootRemovalAllowed = true;
await provider.close();
await releaseFixture?.();
});
@@ -125,8 +139,16 @@ it.runIf(supportedHost)(
await expect(access(ownedPath)).resolves.toBeUndefined();
cleanupAllowed = true;
await provider.close();
await expect(provider.close()).rejects.toMatchObject({
cleanupIncomplete: true,
});
expect(() => process.kill(workerPid, 0)).toThrow();
expect(result.error.cleanupResources).toEqual([ownedPath]);
expect(result.error.cleanupIncomplete).toBe(true);
rootRemovalAllowed = true;
await provider.close();
expect(result.error.cleanupIncomplete).toBe(false);
expect(refused.error.cleanupIncomplete).toBe(false);
await expect(access(ownedPath)).rejects.toMatchObject({ code: "ENOENT" });
},
);
@@ -189,6 +211,7 @@ it.runIf(supportedHost)(
expect(releases).toBe(2);
removalAllowed = true;
await provider.close();
expect(result.error.cleanupIncomplete).toBe(false);
await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" });
},
);
@@ -14,6 +14,7 @@ import {
import { BinaryLayoutService } from "../../src/application/binaryDiagnostics/BinaryLayoutService.js";
import { createCli } from "../../src/cli.js";
import * as composition from "../../src/composition/binaryDiagnostics.js";
import { resolvedAnalysisError } from "../../src/domain/analysisErrorBase.js";
import { projectAnalysisError } from "../../src/domain/analysisErrorProjection.js";
import { PwntoolsLayoutProvider } from "../../src/native/pwntools/PwntoolsLayoutProvider.js";
import type { PwntoolsLauncher } from "../../src/native/pwntools/PwntoolsDecoder.js";
@@ -437,3 +438,50 @@ it.runIf(supportedHost)(
await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" });
},
);
it.runIf(supportedHost)(
"retains cancellation and decoded facts until failed final cleanup is retried",
async () => {
const { path, root, launch } = await fixture();
const controller = new AbortController();
const reason = { stage: "failed final cleanup" };
let removalAllowed = false;
const provider = new PwntoolsLayoutProvider(
{ REA_PWNTOOLS_PYTHON: process.execPath },
launch,
async () => ({
path: root.path,
close: async () => {
if (!removalAllowed) {
controller.abort(reason);
throw new Error("root removal refused");
}
await root.close();
},
}),
);
onTestFinished(async () => {
removalAllowed = true;
await provider.close();
});
const result = await provider.inspectAndClose(
{ path },
{ signal: controller.signal },
);
if (result.ok)
throw new Error("Expected cancelled finalization with retained cleanup");
expect(projectAnalysisError(result.error)).toMatchObject({
code: "cleanup_incomplete",
details: { diagnostics: { primary_error: { code: "cancelled" } } },
});
expect(result.error.cleanupPrimary?.cause).toBe(reason);
expect(result.error.partialObservation).toBeDefined();
expect(result.error.capturedOutput?.stdout).toBe("decoder output");
expect(result.error.cleanupResources).toEqual([root.path]);
removalAllowed = true;
await provider.close();
expect(result.error.cleanupIncomplete).toBe(false);
expect(resolvedAnalysisError(result.error).cause).toBe(reason);
await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" });
},
);
@@ -1,4 +1,4 @@
import { access, mkdir, readFile, writeFile } from "node:fs/promises";
import { access, mkdir, open, readFile, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { fileURLToPath } from "node:url";
import {
@@ -25,6 +25,10 @@ import {
import { prepareProcessOwnershipInspection } from "../../src/process/ProcessOwnershipObservation.js";
import { spawnOwnedProviderProcess } from "../../src/process/ProviderProcess.js";
import { PrivateRuntimeRoot } from "../../src/process/PrivateRuntimeRoot.js";
import {
withArtifactFileHandle,
ArtifactFileCleanupFailure,
} from "../../src/artifacts/ArtifactFileOperation.js";
import * as artifacts from "../../src/artifacts/readStableArtifact.js";
import {
recordedCrashFixture,
@@ -269,12 +273,15 @@ it.runIf(supportedHost).each([
expect(projected.code).toBe("cleanup_incomplete");
if (retainedOwnership === undefined)
throw new Error("Expected retained process ownership");
expect(projected.details?.resources).toEqual([
retainedOwnership.runId,
root.path,
`pid:${retainedOwnership.leaderPid}`,
`process-group:${retainedOwnership.processGroupId}`,
]);
expect(projected.details?.resources).toEqual(
expect.arrayContaining([
retainedOwnership.runId,
root.path,
`pid:${retainedOwnership.leaderPid}`,
`process-group:${retainedOwnership.processGroupId}`,
]),
);
expect(projected.details?.resources).toHaveLength(4);
if (stage === "debugger")
expect(projected).toMatchObject({
details: {
@@ -368,3 +375,104 @@ it.runIf(supportedHost)(
await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" });
},
);
it.runIf(supportedHost).each(["decoder", "debugger"])(
"does not reinterpret a completed %s read when retention cancels after failed close",
async (stage) => {
const { path, root, environment, basic } = await fixture();
const controller = new AbortController();
const reason = { stage: "retention callback after owner completion" };
const reply = join(
root.path,
...(stage === "debugger" ? ["debugger"] : []),
"reply.json",
);
const read = artifacts.readStableArtifact;
let injected = false;
let releaseAllowed = false;
const emergencyCloses: (() => Promise<void>)[] = [];
vi.spyOn(artifacts, "readStableArtifact").mockImplementation(
async (...args) => {
if (args[0] !== reply || injected) return read(...args);
injected = true;
const resources = args[4];
if (resources === undefined)
throw new Error("Reply must have an artifact resource owner");
const file = await open(reply, "r");
const close = file.close.bind(file);
emergencyCloses.push(close);
file.close = async () => {
if (!releaseAllowed) throw new Error("reply close refused");
await close();
};
const retain = resources.retain.bind(resources);
const spy = vi
.spyOn(resources, "retain")
.mockImplementation((owner) => {
retain(owner);
controller.abort(reason);
});
try {
return await withArtifactFileHandle(
reply,
file,
resources,
controller.signal,
() => read(...args),
);
} finally {
spy.mockRestore();
}
},
);
let launches = 0;
const provider = new PwntoolsRecordedCrashProvider(
environment,
async (spawn) => {
if (++launches === 1) return basic(spawn);
await writeFile(
reply,
JSON.stringify({ ok: true, value: recordedCrashDebuggerFixture() }),
);
return spawnOwnedProviderProcess({
...spawn,
arguments: ["-e", "process.stdout.write('debugger diagnostic')"],
});
},
async () => root,
);
onTestFinished(async () => {
releaseAllowed = true;
try {
await provider.close();
} finally {
await Promise.all(emergencyCloses.map((close) => close()));
}
});
const result = await provider.inspectAndClose(
{ path, include_debugger_context: true },
{ signal: controller.signal },
);
if (result.ok) throw new Error("Expected the physical reply-close failure");
expect(result.error._tag).toBe("AnalysisOutputError");
expect(result.error.cause).toBeInstanceOf(ArtifactFileCleanupFailure);
if (!(result.error.cause instanceof ArtifactFileCleanupFailure))
throw new Error("Expected exact file owner");
expect(result.error.cause.fileCleanupFailure.outcome.kind).toBe(
"completed",
);
expect(
result.error.cause.fileCleanupFailure.completionCancellation,
).toBeUndefined();
expect(result.error.cleanupIncomplete).toBe(true);
await expect(provider.close()).rejects.toMatchObject({
cleanupIncomplete: true,
});
await expect(access(root.path)).resolves.toBeUndefined();
releaseAllowed = true;
await provider.close();
expect(result.error.cleanupIncomplete).toBe(false);
expect(result.error.capturedOutput?.stdout).toContain("basic diagnostic");
await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" });
},
);