mirror of
https://github.com/morluto/rea.git
synced 2026-11-06 14:56:32 +00:00
fix(native): retain failed command cleanup owners
Retain failed native command owners through both xcrun resolution and command capture. Retry all owned cleanup failures before new commands and after consumers join their launched work at close; preserve partial output and every resource diagnostic. Refs #740.
This commit is contained in:
1 parent
8be7ad496c
commit
d9b4ae182d
6 files changed
+724
-41
No files matched your search
@@ -1,3 +1,5 @@
|
||||
import { EventEmitter } from "node:events";
|
||||
import { PassThrough } from "node:stream";
|
||||
import { writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
|
||||
@@ -6,9 +8,151 @@ import { describe, expect, it, onTestFinished } from "vitest";
|
||||
import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js";
|
||||
import { CPU, fatImage, machoImage } from "./apple/MachoImage.fixture.js";
|
||||
import { MachOSliceArtifactReader } from "./MachOSliceArtifactReader.js";
|
||||
import { ok } from "../domain/result.js";
|
||||
import { err, ok } from "../domain/result.js";
|
||||
import { openRegularFile } from "../filesystem/RegularFile.js";
|
||||
import { ProviderProcessSupervisor } from "../process/ProviderProcess.js";
|
||||
import type { NativeCommandRunner } from "../native/CommandRunner.js";
|
||||
import { NativeCommandFailure } from "../native/CommandRunner.js";
|
||||
|
||||
const cleanupOwner = (runId: string): ProviderProcessSupervisor => {
|
||||
const child = Object.assign(new EventEmitter(), {
|
||||
stdout: new PassThrough(),
|
||||
stderr: new PassThrough(),
|
||||
exitCode: null as number | null,
|
||||
signalCode: null as NodeJS.Signals | null,
|
||||
kill: () => true,
|
||||
});
|
||||
return new ProviderProcessSupervisor({
|
||||
process: child,
|
||||
ownsProcessLifetime: true,
|
||||
ownership: {
|
||||
runId,
|
||||
leaderPid: 45_000,
|
||||
processGroupId: 45_000,
|
||||
expectedParentPid: process.pid,
|
||||
},
|
||||
cleanup: async () => ({ cleaned: false, reason: "still running" }),
|
||||
});
|
||||
};
|
||||
|
||||
describe("Mach-O cleanup exceptions", () => {
|
||||
it("retains an undefined cleanup rejection as an explicit failure", async () => {
|
||||
const reader = new MachOSliceArtifactReader(
|
||||
"/unused/macho",
|
||||
{},
|
||||
{
|
||||
run: async () => {
|
||||
throw new Error("Unexpected command");
|
||||
},
|
||||
retryCleanup: async () => {
|
||||
throw undefined;
|
||||
},
|
||||
},
|
||||
);
|
||||
await expect(reader.close()).rejects.toMatchObject({
|
||||
cleanup: {
|
||||
reason: "undefined",
|
||||
resources: ["native-command-process-cleanup"],
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("Mach-O command cleanup lifecycle", () => {
|
||||
it("waits for active lipo capture before cleanup and preserves its failure", async () => {
|
||||
const root = await createTestTempDirectory("rea-slices-close-race-");
|
||||
const binary = join(root, "input");
|
||||
await writeFile(binary, Buffer.from("fixture"));
|
||||
let finishCommand!: (
|
||||
result: Awaited<ReturnType<NativeCommandRunner["run"]>>,
|
||||
) => void;
|
||||
let commandStarted!: () => void;
|
||||
const started = new Promise<void>((resolve) => {
|
||||
commandStarted = resolve;
|
||||
});
|
||||
const command = new Promise<
|
||||
Awaited<ReturnType<NativeCommandRunner["run"]>>
|
||||
>((resolve) => {
|
||||
finishCommand = resolve;
|
||||
});
|
||||
let cleanupRetries = 0;
|
||||
const runner: NativeCommandRunner = {
|
||||
run: () => {
|
||||
commandStarted();
|
||||
return command;
|
||||
},
|
||||
retryCleanup: async () => {
|
||||
cleanupRetries += 1;
|
||||
if (cleanupRetries > 1) return [];
|
||||
return [
|
||||
{
|
||||
owner: cleanupOwner("run:first"),
|
||||
reason: "first process group remains live",
|
||||
resources: ["run:first", "process-group:1234"],
|
||||
},
|
||||
{
|
||||
owner: cleanupOwner("run:second"),
|
||||
reason: "second process group remains live",
|
||||
resources: ["run:second", "process-group:5678"],
|
||||
},
|
||||
];
|
||||
},
|
||||
};
|
||||
const reader = new MachOSliceArtifactReader(binary, {}, runner);
|
||||
const iteration = reader.entries();
|
||||
const firstEntry = iteration[Symbol.asyncIterator]().next();
|
||||
await started;
|
||||
const closing = reader.close();
|
||||
await Promise.resolve();
|
||||
expect(cleanupRetries).toBe(0);
|
||||
finishCommand(
|
||||
err(
|
||||
new NativeCommandFailure(
|
||||
"lipo",
|
||||
"timeout",
|
||||
null,
|
||||
undefined,
|
||||
{
|
||||
stdout: "partial lipo output",
|
||||
stderr: "partial lipo diagnostic",
|
||||
stdoutBytes: 19,
|
||||
stderrBytes: 23,
|
||||
exitCode: null,
|
||||
signal: null,
|
||||
truncated: false,
|
||||
},
|
||||
"capture process group remains unverified",
|
||||
["capture-group:9999"],
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
await expect(firstEntry).rejects.toMatchObject({
|
||||
reason: "unavailable",
|
||||
cause: expect.objectContaining({
|
||||
cleanupFailure: "capture process group remains unverified",
|
||||
}),
|
||||
cleanup: {
|
||||
reason: "capture process group remains unverified",
|
||||
resources: ["capture-group:9999"],
|
||||
},
|
||||
});
|
||||
await expect(closing).rejects.toMatchObject({
|
||||
cleanup: {
|
||||
reason:
|
||||
"run:first, process-group:1234: first process group remains live; run:second, process-group:5678: second process group remains live",
|
||||
resources: [
|
||||
"run:first",
|
||||
"process-group:1234",
|
||||
"run:second",
|
||||
"process-group:5678",
|
||||
],
|
||||
},
|
||||
});
|
||||
await reader.close();
|
||||
expect(cleanupRetries).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("MachOSliceArtifactReader", () => {
|
||||
it("keeps a borrowed source usable after a cancelled slice read", async () => {
|
||||
|
||||
@@ -39,6 +39,7 @@ export class MachOSliceArtifactReader implements ArtifactReader {
|
||||
#ownedSource: OwnedFileHandle | undefined;
|
||||
#pendingSource: Promise<StableRegularFileDescriptor> | undefined;
|
||||
readonly #entries = new Map<string, ArtifactEntry>();
|
||||
readonly #pendingCommands = new Set<Promise<unknown>>();
|
||||
readonly #ownsSource: boolean;
|
||||
#closed = false;
|
||||
#closePromise: Promise<void> | undefined;
|
||||
@@ -59,16 +60,46 @@ export class MachOSliceArtifactReader implements ArtifactReader {
|
||||
this.#entries.clear();
|
||||
const source = await this.#ensureSource();
|
||||
await this.#verifySource(source);
|
||||
const captured = await this.runner.run(
|
||||
if (this.#closed)
|
||||
throw new ArtifactReaderFailure(
|
||||
"unavailable",
|
||||
"Universal Mach-O reader is closed",
|
||||
);
|
||||
const command = this.runner.run(
|
||||
"lipo",
|
||||
["-detailed_info", this.path],
|
||||
signal === undefined ? {} : { signal },
|
||||
);
|
||||
this.#pendingCommands.add(command);
|
||||
let captured: Awaited<typeof command>;
|
||||
try {
|
||||
captured = await command;
|
||||
} finally {
|
||||
this.#pendingCommands.delete(command);
|
||||
}
|
||||
if (!captured.ok)
|
||||
throw new ArtifactReaderFailure(
|
||||
captured.error.reason === "cancelled" ? "cancelled" : "unavailable",
|
||||
"lipo could not enumerate universal Mach-O slices",
|
||||
{ cause: captured.error },
|
||||
{
|
||||
cause: captured.error,
|
||||
...(captured.error.cleanupFailure === null &&
|
||||
captured.error.cleanupResources.length === 0
|
||||
? {}
|
||||
: {
|
||||
cleanup: {
|
||||
reason:
|
||||
captured.error.cleanupFailure ??
|
||||
"Native command cleanup remains unverified",
|
||||
resources: [...captured.error.cleanupResources],
|
||||
},
|
||||
}),
|
||||
},
|
||||
);
|
||||
if (this.#closed)
|
||||
throw new ArtifactReaderFailure(
|
||||
"unavailable",
|
||||
"Universal Mach-O reader closed during slice discovery",
|
||||
);
|
||||
this.#command = {
|
||||
tool: captured.value.tool,
|
||||
@@ -262,13 +293,49 @@ export class MachOSliceArtifactReader implements ArtifactReader {
|
||||
|
||||
async close(): Promise<void> {
|
||||
this.#closed = true;
|
||||
this.#closePromise ??= this.#closeSource().catch((cause: unknown) => {
|
||||
this.#closePromise ??= this.#releaseResources().catch((cause: unknown) => {
|
||||
this.#closePromise = undefined;
|
||||
throw cause;
|
||||
});
|
||||
return this.#closePromise;
|
||||
}
|
||||
|
||||
async #releaseResources(): Promise<void> {
|
||||
await Promise.allSettled([...this.#pendingCommands]);
|
||||
let cleanup: ArtifactReaderFailure["cleanup"];
|
||||
try {
|
||||
const failures = (await this.runner.retryCleanup?.()) ?? [];
|
||||
if (failures.length > 0)
|
||||
cleanup = {
|
||||
reason: failures
|
||||
.map(
|
||||
({ reason, resources }) => `${resources.join(", ")}: ${reason}`,
|
||||
)
|
||||
.join("; "),
|
||||
resources: [
|
||||
...new Set(failures.flatMap(({ resources }) => resources)),
|
||||
],
|
||||
};
|
||||
} catch (cause: unknown) {
|
||||
cleanup = ArtifactReaderFailure.cleanupObservation(
|
||||
cause,
|
||||
"native-command-process-cleanup",
|
||||
);
|
||||
}
|
||||
try {
|
||||
await this.#closeSource();
|
||||
} catch (cause: unknown) {
|
||||
if (cleanup === undefined) throw cause;
|
||||
throw ArtifactReaderFailure.withCleanup(cause, cleanup);
|
||||
}
|
||||
if (cleanup !== undefined)
|
||||
throw new ArtifactReaderFailure(
|
||||
"unavailable",
|
||||
"Native command process cleanup remains incomplete",
|
||||
{ cleanup },
|
||||
);
|
||||
}
|
||||
|
||||
async #ensureSource(): Promise<StableRegularFileDescriptor> {
|
||||
if (this.#closed)
|
||||
throw new ArtifactReaderFailure(
|
||||
|
||||
Reference in new issue
Block a user