fix(artifacts)!: preserve descriptor cleanup authority

Keep rejected closes unresolved after descriptor invalidation and share concurrent close attempts. Preserve primary admission failures and transfer the exact failed handle owner into inventory scopes.

BREAKING CHANGE: ArtifactResourceScope file-handle owners require OwnedFileHandle. Root inventory sources retain their single cleanup owner.
This commit is contained in:
morluto committed 2026-10-11 05:17:10 +08:00
1 parent 32d13c431e
commit be6272f261
10 files changed
+443 -57

No files matched your search

+2 -2
View File
@@ -1,4 +1,4 @@
import type { FileHandle } from "node:fs/promises";
import type { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js";
import type { ArtifactReader } from "./ArtifactReader.js";
import { ArtifactReaderFailure } from "./ArtifactReader.js";
@@ -14,7 +14,7 @@ export type ArtifactResourceOwner =
| {
readonly kind: "file-handle";
readonly resource: string;
readonly handle: FileHandle;
readonly handle: OwnedFileHandle;
}
| {
readonly kind: "output-tree";
+57
View File
@@ -0,0 +1,57 @@
import { expect, it, vi } from "vitest";
import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js";
it("shares one in-flight successful close between concurrent owners", async () => {
let resolveClose: (() => void) | undefined;
const file = {
fd: 17,
close: vi.fn(
() =>
new Promise<void>((resolve) => {
file.fd = -1;
resolveClose = resolve;
}),
),
};
const owner = new OwnedFileHandle(file);
const first = owner.close();
const second = owner.close();
await Promise.resolve();
expect(file.close).toHaveBeenCalledTimes(1);
resolveClose?.();
await expect(Promise.all([first, second])).resolves.toEqual([
undefined,
undefined,
]);
await expect(owner.close()).resolves.toBeUndefined();
expect(file.close).toHaveBeenCalledTimes(1);
});
it("shares one in-flight failed close and keeps its unconfirmed result sticky", async () => {
const failure = new Error("native close failed");
let rejectClose: (() => void) | undefined;
const file = {
fd: 17,
close: vi.fn(
() =>
new Promise<void>((_resolve, reject) => {
file.fd = -1;
rejectClose = () => reject(failure);
}),
),
};
const owner = new OwnedFileHandle(file);
const first = owner.close();
const second = owner.close();
await Promise.resolve();
expect(file.close).toHaveBeenCalledTimes(1);
rejectClose?.();
await expect(Promise.all([first, second])).rejects.toBe(failure);
await expect(owner.close()).rejects.toBe(failure);
expect(file.close).toHaveBeenCalledTimes(1);
});
@@ -0,0 +1,68 @@
import type { Stats } from "node:fs";
import type { FileHandle } from "node:fs/promises";
import { afterEach, expect, it, vi } from "vitest";
import {
NonRegularFileReadError,
openRegularFile,
RegularFileAdmissionFailure,
} from "../filesystem/RegularFile.js";
const { mockOpen } = vi.hoisted(() => ({ mockOpen: vi.fn() }));
vi.mock("node:fs/promises", async (importOriginal) => ({
...(await importOriginal<typeof import("node:fs/promises")>()),
open: mockOpen,
}));
afterEach(() => mockOpen.mockReset());
it.each(["cancellation after open", "non-regular metadata", "stat failure"])(
"retains the exact owner when close fails after %s",
async (failureCase) => {
const closeFailure = new Error("native close failed");
const statFailure = new Error("descriptor stat failed");
const cancellation = new Error("cancelled after open");
const controller = new AbortController();
const file = {
fd: 17,
stat: vi.fn(async () => {
if (failureCase === "cancellation after open") {
controller.abort(cancellation);
return { isFile: () => true } as Stats;
}
if (failureCase === "non-regular metadata")
return {
isFile: () => false,
isDirectory: () => false,
} as Stats;
throw statFailure;
}),
close: vi.fn(async () => {
file.fd = -1;
throw closeFailure;
}),
};
mockOpen.mockResolvedValue(file as unknown as FileHandle);
const cause = await openRegularFile("/selected/input", {
symlinks: "reject",
signal: controller.signal,
}).catch((failure: unknown) => failure);
expect(cause).toBeInstanceOf(RegularFileAdmissionFailure);
if (!(cause instanceof RegularFileAdmissionFailure))
throw new Error("Expected an admission failure that retains its owner");
expect(cause.owner.handle).toBe(file);
expect(cause.cleanupCause).toBe(closeFailure);
expect(file.close).toHaveBeenCalledTimes(1);
await expect(cause.owner.close()).rejects.toBe(closeFailure);
expect(file.close).toHaveBeenCalledTimes(1);
if (failureCase === "non-regular metadata")
expect(cause.cause).toBeInstanceOf(NonRegularFileReadError);
else if (failureCase === "stat failure")
expect(cause.cause).toBe(statFailure);
else expect(cause.cause).toBe(cancellation);
},
);
+23 -28
View File
@@ -1,11 +1,8 @@
import {
NonRegularFileReadError,
openRegularFile,
sameRegularFileState,
} from "../../filesystem/RegularFile.js";
import { sameRegularFileState } from "../../filesystem/RegularFile.js";
import type { FileHandle } from "node:fs/promises";
import type { Stats } from "node:fs";
import type { StableRegularFileDescriptor } from "../../filesystem/RegularFile.js";
import type { OwnedFileHandle } from "../../filesystem/OwnedFileHandle.js";
import { classifyArtifactContent } from "./ArtifactGraphConstruction.js";
import { ARTIFACT_CLASSIFICATION_PREFIX_BYTES } from "../ArtifactHash.js";
@@ -17,7 +14,10 @@ import {
import { ArtifactReaderFailure } from "../ArtifactReader.js";
import { type ArtifactResourceOwner } from "../ArtifactResourceScope.js";
import type { HashResult } from "../ArtifactHash.js";
import { hashStableRootArtifactHandle } from "./hashStableRootArtifact.js";
import {
hashStableRootArtifactHandle,
openRootArtifact,
} from "./hashStableRootArtifact.js";
import type { ArtifactInventoryOptions } from "./types.js";
interface RootClassification {
@@ -26,9 +26,14 @@ interface RootClassification {
}
type RootInventoryClassification = RootClassification & {
readonly rootSource: StableRegularFileDescriptor | undefined;
readonly rootSource: RootInventorySource | undefined;
};
/** The admitted descriptor and the single owner that can confirm its close. */
export interface RootInventorySource extends StableRegularFileDescriptor {
readonly owner: OwnedFileHandle;
}
/** Classify and hash one file root through the same stable open descriptor. */
export const classifyAndHashRoot = async (
path: string,
@@ -47,7 +52,7 @@ export const classifyAndHashRoot = async (
if (rootSource !== undefined) {
const owner: ArtifactResourceOwner = {
kind: "file-handle",
handle: rootSource.handle,
handle: rootSource.owner,
resource: `root artifact descriptor for ${path}`,
};
const cleanupAttempt = await options.resourceScope.release(owner);
@@ -73,10 +78,15 @@ export const classifyAndHashRootForInventory = async (
): Promise<RootInventoryClassification> => {
if (directory)
return { format: "directory", digest: null, rootSource: undefined };
const handle = await openRootFile(path, options.signal);
const ownedHandle = await openRootArtifact(
path,
options.resourceScope,
options.signal,
);
const handle = ownedHandle.handle;
const owner: ArtifactResourceOwner = {
kind: "file-handle",
handle,
handle: ownedHandle,
resource: `root artifact descriptor for ${path}`,
};
let outcome:
@@ -105,7 +115,9 @@ export const classifyAndHashRootForInventory = async (
value: {
format,
digest,
rootSource: retainSource ? { handle, initial } : undefined,
rootSource: retainSource
? { handle, initial, owner: ownedHandle }
: undefined,
},
};
} catch (cause: unknown) {
@@ -133,23 +145,6 @@ const isZipFormat = (format: ArtifactOccurrence["artifact_format"]): boolean =>
format === "msix" ||
format === "appx";
const openRootFile = async (
path: string,
signal?: AbortSignal,
): Promise<FileHandle> => {
try {
return await openRegularFile(path, { symlinks: "reject", signal });
} catch (cause: unknown) {
if (cause instanceof NonRegularFileReadError)
throw new ArtifactReaderFailure(
"format",
`Artifact root is not a regular file: ${path}`,
{ cause },
);
throw cause;
}
};
const classifyRootFormat = async (
path: string,
handle: FileHandle,
@@ -1,10 +1,13 @@
import { lstat } from "node:fs/promises";
import type { Stats } from "node:fs";
import type { FileHandle } from "node:fs/promises";
import { readFileHandleChunks } from "../../filesystem/readFileHandleChunks.js";
import { OwnedFileHandle } from "../../filesystem/OwnedFileHandle.js";
import { ArtifactReaderFailure } from "../ArtifactReader.js";
import {
NonRegularFileReadError,
RegularFileAdmissionFailure,
openRegularFile,
sameRegularFileState,
} from "../../filesystem/RegularFile.js";
@@ -13,33 +16,79 @@ import {
hashReadable,
type HashResult,
} from "../ArtifactHash.js";
import type { ArtifactResourceScope } from "../ArtifactResourceScope.js";
/** Admit a root descriptor without losing ownership when validation cleanup fails. */
export const openRootArtifact = async (
path: string,
resourceScope: ArtifactResourceScope,
signal?: AbortSignal,
): Promise<OwnedFileHandle> => {
try {
return new OwnedFileHandle(
await openRegularFile(path, { symlinks: "reject", signal }),
);
} catch (cause: unknown) {
const primary =
cause instanceof RegularFileAdmissionFailure ? cause.cause : cause;
const failure =
primary instanceof NonRegularFileReadError
? new ArtifactReaderFailure(
"format",
`Artifact root is not a regular file: ${path}`,
{ cause: primary },
)
: primary;
if (cause instanceof RegularFileAdmissionFailure) {
const resource = `root artifact descriptor for ${path}`;
const cleanup = await resourceScope.release({
kind: "file-handle",
resource,
handle: cause.owner,
});
if (cleanup.kind === "failed")
throw ArtifactReaderFailure.withCleanup(
failure,
ArtifactReaderFailure.cleanupObservation(cleanup.cause, resource),
);
}
throw failure;
}
};
/** Hash one stable regular root file from the same descriptor that was checked. */
export const hashStableRootArtifact = async (
path: string,
resourceScope: ArtifactResourceScope,
signal?: AbortSignal,
): Promise<HashResult> => {
let handle: FileHandle;
try {
handle = await openRegularFile(path, {
symlinks: "reject",
signal,
});
} catch (cause: unknown) {
if (cause instanceof NonRegularFileReadError)
throw new ArtifactReaderFailure(
"format",
`Artifact root is not a regular file: ${path}`,
{ cause },
);
throw cause;
}
const owner = await openRootArtifact(path, resourceScope, signal);
const handle = owner.handle;
let outcome:
| { readonly kind: "completed"; readonly value: HashResult }
| { readonly kind: "failed"; readonly cause: unknown };
try {
const initial = await handle.stat();
return await hashStableRootArtifactHandle(path, handle, initial, signal);
} finally {
await handle.close();
outcome = {
kind: "completed",
value: await hashStableRootArtifactHandle(path, handle, initial, signal),
};
} catch (cause: unknown) {
outcome = { kind: "failed", cause };
}
const resource = `root artifact descriptor for ${path}`;
const cleanup = await resourceScope.release({
kind: "file-handle",
resource,
handle: owner,
});
if (cleanup.kind === "failed")
throw ArtifactReaderFailure.withCleanup(
outcome.kind === "failed" ? outcome.cause : cleanup.cause,
ArtifactReaderFailure.cleanupObservation(cleanup.cause, resource),
);
if (outcome.kind === "failed") throw outcome.cause;
return outcome.value;
};
/** Hash and revalidate the same open descriptor used for root classification. */
@@ -52,7 +101,7 @@ export const hashStableRootArtifactHandle = async (
const digest = await hashReadable(
// Read at most the admitted extent plus one byte to detect growth without
// following an actively appended file indefinitely.
handle.createReadStream({ autoClose: false, start: 0, end: initial.size }),
readFileHandleChunks(handle, { start: 0, end: initial.size }),
signal,
);
abortIfNeeded(signal);
+16 -7
View File
@@ -5,7 +5,7 @@ import { AsarArtifactReader } from "../AsarArtifactReader.js";
import { lstat } from "node:fs/promises";
import type { Stats } from "node:fs";
import type { StableRegularFileDescriptor } from "../../filesystem/RegularFile.js";
import type { RootInventorySource } from "./classify.js";
import {
ArtifactReaderFailure,
@@ -107,6 +107,7 @@ export const scanCanonicalArtifactInventoryInScope = async (
rootFormat,
rootDigest,
rootSource,
resourceScope: options.resourceScope,
reader,
signal: options.signal,
nodes,
@@ -146,7 +147,7 @@ const cleanupInventoryOwners = async ({
}: {
readonly path: string;
readonly options: ArtifactInventoryOptions;
readonly rootSource: StableRegularFileDescriptor | undefined;
readonly rootSource: RootInventorySource | undefined;
readonly ownedReaders: readonly ArtifactReader[];
readonly outcome: InventoryScanOutcome;
}): Promise<ArtifactReaderFailure | undefined> => {
@@ -161,7 +162,7 @@ const cleanupInventoryOwners = async ({
if (rootSource !== undefined)
owners.push({
kind: "file-handle",
handle: rootSource.handle,
handle: rootSource.owner,
resource: `root artifact descriptor for ${path}`,
});
for (const owner of owners) {
@@ -182,11 +183,12 @@ const cleanupInventoryOwners = async ({
};
interface SnapshotBuildInput {
readonly resourceScope: ArtifactInventoryOptions["resourceScope"];
readonly path: string;
readonly metadata: Stats;
readonly rootFormat: ArtifactOccurrence["artifact_format"];
readonly rootDigest: HashResult | null;
readonly rootSource: StableRegularFileDescriptor | undefined;
readonly rootSource: RootInventorySource | undefined;
readonly reader: ArtifactReader | undefined;
readonly signal: AbortSignal | undefined;
readonly nodes: Map<string, ArtifactNode>;
@@ -243,7 +245,13 @@ const buildInventorySnapshot = async (
ordinal,
}));
await verifyRootDigest(path, rootDigest, rootSource, signal);
await verifyRootDigest(
path,
rootDigest,
rootSource,
input.resourceScope,
signal,
);
const graphSha256 = artifactGraphDigest({
nodes: orderedNodes,
@@ -318,13 +326,14 @@ const buildIntegrityContradictions = (
const verifyRootDigest = async (
path: string,
rootDigest: HashResult | null,
rootSource: StableRegularFileDescriptor | undefined,
rootSource: RootInventorySource | undefined,
resourceScope: ArtifactInventoryOptions["resourceScope"],
signal: AbortSignal | undefined,
): Promise<void> => {
if (rootDigest === null) return;
const verified =
rootSource === undefined
? await hashStableRootArtifact(path, signal)
? await hashStableRootArtifact(path, resourceScope, signal)
: await hashStableRootArtifactHandle(
path,
rootSource.handle,
+45
View File
@@ -0,0 +1,45 @@
import type { FileHandle } from "node:fs/promises";
type CloseState =
| { readonly kind: "open" }
| { readonly kind: "closing"; readonly attempt: Promise<void> }
| { readonly kind: "closed" }
| { readonly kind: "failed"; readonly cause: unknown };
/** One file handle whose close result must remain truthful across retries. */
export class OwnedFileHandle<
Handle extends Pick<FileHandle, "fd" | "close"> = FileHandle,
> {
#state: CloseState = { kind: "open" };
constructor(readonly handle: Handle) {}
/** Close the owned handle, retaining an unconfirmed close as a sticky failure. */
close(): Promise<void> {
if (this.#state.kind === "closed") return Promise.resolve();
if (this.#state.kind === "closing") return this.#state.attempt;
if (this.#state.kind === "failed") return Promise.reject(this.#state.cause);
if (this.handle.fd < 0) {
const cause = new Error(
"File handle was invalidated before this owner confirmed its close",
);
this.#state = { kind: "failed", cause };
return Promise.reject(cause);
}
const attempt = Promise.resolve().then(() => this.#attemptClose());
this.#state = { kind: "closing", attempt };
return attempt;
}
async #attemptClose(): Promise<void> {
try {
await this.handle.close();
this.#state = { kind: "closed" };
} catch (cause: unknown) {
this.#state =
this.handle.fd < 0 ? { kind: "failed", cause } : { kind: "open" };
throw cause;
}
}
}
+26 -1
View File
@@ -2,6 +2,8 @@ import { constants } from "node:fs";
import type { BigIntStats, Stats } from "node:fs";
import { open, type FileHandle } from "node:fs/promises";
import { OwnedFileHandle } from "./OwnedFileHandle.js";
/** Identifies a selected path that cannot be read as one regular file. */
export class NonRegularFileReadError extends Error {
readonly code: "EISDIR" | "ENOTFILE";
@@ -16,6 +18,24 @@ export class NonRegularFileReadError extends Error {
}
}
/** Admission failed and the opened descriptor could not be confirmed closed. */
export class RegularFileAdmissionFailure extends Error {
override readonly cause: unknown;
constructor(
readonly path: string,
admissionCause: unknown,
readonly cleanupCause: unknown,
readonly owner: OwnedFileHandle,
) {
super(
`Selected input failed admission and its descriptor close failed: ${path}`,
);
this.name = "RegularFileAdmissionFailure";
this.cause = admissionCause;
}
}
/** A regular input no longer matches the extent or identity admitted for reading. */
export class RegularFileChangedError extends Error {
readonly code = "ESTALE";
@@ -47,6 +67,7 @@ export const openRegularFile = async (
(constants.O_NOCTTY ?? 0) |
(options.symlinks === "reject" ? (constants.O_NOFOLLOW ?? 0) : 0),
);
const owner = new OwnedFileHandle(handle);
try {
options.signal?.throwIfAborted();
const metadata = await handle.stat();
@@ -55,7 +76,11 @@ export const openRegularFile = async (
throw new NonRegularFileReadError(path, metadata.isDirectory());
return handle;
} catch (cause: unknown) {
await handle.close().catch(() => undefined);
try {
await owner.close();
} catch (cleanupCause: unknown) {
throw new RegularFileAdmissionFailure(path, cause, cleanupCause, owner);
}
throw cause;
}
};
+46
View File
@@ -0,0 +1,46 @@
import type { FileHandle } from "node:fs/promises";
import { Readable } from "node:stream";
/** Stream positional reads while leaving descriptor cleanup to its owner. */
export const readFileHandleChunks = (
handle: Pick<FileHandle, "read">,
options: {
readonly start?: number;
readonly end?: number;
readonly signal?: AbortSignal | undefined;
} = {},
): Readable => {
const start = options.start ?? 0;
const end = options.end ?? Infinity;
if (
!Number.isSafeInteger(start) ||
start < 0 ||
(end !== Infinity && (!Number.isSafeInteger(end) || end < start))
)
throw new RangeError("Invalid file read range");
return Readable.from(
(async function* () {
let position = start;
while (position <= end) {
options.signal?.throwIfAborted();
const chunk = Buffer.allocUnsafe(
Math.min(64 * 1024, end - position + 1),
);
const { bytesRead } = await handle.read(
chunk,
0,
chunk.length,
position,
);
options.signal?.throwIfAborted();
if (bytesRead === 0) return;
position += bytesRead;
yield chunk.subarray(0, bytesRead);
}
})(),
{
objectMode: false,
...(options.signal === undefined ? {} : { signal: options.signal }),
},
);
};
@@ -0,0 +1,92 @@
import type { FileHandle } from "node:fs/promises";
import { writeFile } from "node:fs/promises";
import { join } from "node:path";
import { expect, it, onTestFinished, vi } from "vitest";
import { ArtifactProvider } from "../../../src/artifacts/ArtifactProvider.js";
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
const injected = vi.hoisted(() => ({
opened: undefined as
| ((path: unknown, handle: FileHandle) => void)
| undefined,
}));
vi.mock("node:fs/promises", async (importOriginal) => {
const actual = await importOriginal<typeof import("node:fs/promises")>();
return {
...actual,
open: async (...args: Parameters<typeof actual.open>) => {
const handle = await actual.open(...args);
injected.opened?.(args[0], handle);
return handle;
},
};
});
it.each([false, true])(
"preserves root admission failure and close authority (invalidated=%s)",
async (invalidated) => {
const root = await createTestTempDirectory("rea-root-admission-cleanup-");
const path = join(root, "payload.bin");
await writeFile(path, "selected bytes");
const client = new ArtifactProvider({}).createClient({
path,
sourcePath: path,
sha256: "0".repeat(64),
kind: "artifact",
format: "javascript",
});
let descriptor: FileHandle | undefined;
let closeAttempts = 0;
let blocked = true;
injected.opened = (openedPath, handle) => {
if (openedPath !== path || descriptor !== undefined) return;
descriptor = handle;
const close = handle.close.bind(handle);
vi.spyOn(handle, "stat").mockRejectedValueOnce(
Object.assign(new Error("primary admission stat failure"), {
code: "EIO",
errno: -5,
syscall: "fstat",
}),
);
vi.spyOn(handle, "close").mockImplementation(async () => {
closeAttempts += 1;
if (invalidated) await close();
if (blocked) throw new Error("descriptor cleanup outcome unavailable");
await close();
});
};
onTestFinished(async () => {
injected.opened = undefined;
blocked = false;
await client.close();
vi.restoreAllMocks();
});
const inventory = await client.execute("inventory_artifact", {});
expect(inventory).toMatchObject({
ok: false,
error: {
reason: "io",
detail: expect.stringContaining("primary admission stat failure"),
cleanup: {
resources: [expect.stringContaining(path)],
reason: "descriptor cleanup outcome unavailable",
},
},
});
expect(descriptor).toBeDefined();
expect((await client.close()).ok).toBe(false);
const attemptsBeforeRecovery = closeAttempts;
blocked = false;
const recovery = await client.close();
expect(recovery.ok).toBe(!invalidated);
expect(closeAttempts).toBe(
invalidated ? attemptsBeforeRecovery : attemptsBeforeRecovery + 1,
);
expect(descriptor?.fd).toBe(-1);
if (invalidated) expect(closeAttempts).toBe(1);
},
);