fix(process): retain kernel limit descriptor cleanup ownership

Use the shared artifact file owner for kernel report reads, preserving the
physical outcome and exact retry lease through failed close and recovery.
Remove the handwritten finally path that discarded this authority.

Cover successful and failed reads with and without a caller scope, denied
retry, blocked replacement admission, and recovery of original error markers.
Adapt the separate descriptor repair proposed in #4863.

Fixes #4926
Co-authored-by: abdooo2235 <abdalrhmantheiv15@gmail.com>
This commit is contained in:
morluto committed 2026-10-18 08:29:57 +08:00
1 parent 93196449ad
commit 2183c9598c
2 files changed
+130 -27

No files matched your search

+121 -1
View File
@@ -1,9 +1,40 @@
import { expect, it } from "vitest";
import { expect, it, onTestFinished, vi } from "vitest";
import { writeFile } from "node:fs/promises";
import { join } from "node:path";
import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js";
import { ArtifactFileCleanupFailure } from "../artifacts/ArtifactFileOperation.js";
import { ArtifactAnalysisResources } from "./ArtifactAnalysisResources.js";
import { readStableArtifact } from "../artifacts/readStableArtifact.js";
import { ok } from "../domain/result.js";
import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
import {
BOUNDED_PRODUCER_LIMITS,
clampLinuxResourceLimits,
prlimitArgumentVector,
readLinuxResourceLimits,
} from "./LinuxResourceLimits.js";
const kernel = vi.hoisted(() => ({ path: "", denied: false, opens: 0 }));
vi.mock("node:fs/promises", async (importOriginal) => {
const filesystem = await importOriginal<typeof import("node:fs/promises")>();
return {
...filesystem,
open: async (...args: Parameters<typeof filesystem.open>) => {
if (args[0] !== "/proc/self/limits" || kernel.path === "")
return filesystem.open(...args);
const file = await filesystem.open(kernel.path, args[1], args[2]);
kernel.opens += 1;
const close = file.close.bind(file);
// Keep fixture ownership even when an assertion interrupts retry checks.
onTestFinished(close);
file.close = async () => {
if (kernel.denied) throw new Error("Kernel descriptor close denied");
await close();
};
return file;
},
};
});
const configured = {
address_space_bytes: 1073741824,
file_bytes: 67108864,
@@ -80,3 +111,92 @@ it("pairs each effective ceiling with itself before the producer argv", () => {
"/bin/producer",
]);
});
it.each([
["completed", true],
["failed", true],
["completed", false],
["failed", false],
] as const)(
"retains exact cleanup ownership after a %s read (scoped=%s)",
async (outcome, scoped) => {
const directory = await createTestTempDirectory("rea-kernel-limits-");
kernel.path = join(directory, "limits");
kernel.denied = true;
const scope = new ArtifactResourceScope();
onTestFinished(async () => {
kernel.denied = false;
await scope.close();
kernel.path = "";
});
await writeFile(
kernel.path,
outcome === "completed" ? report("unlimited unlimited") : "invalid",
);
const failure: unknown = await readLinuxResourceLimits(
configured,
scoped ? scope : undefined,
).catch((cause: unknown) => cause);
expect(failure).toBeInstanceOf(ArtifactFileCleanupFailure);
if (!(failure instanceof ArtifactFileCleanupFailure))
throw new Error("Expected exact descriptor cleanup ownership");
const physical = failure.fileCleanupFailure;
expect(physical.outcome.kind).toBe(outcome);
if (physical.outcome.kind === "completed")
expect(physical.outcome.value).toEqual(configured);
else
expect(physical.outcome.cause).toMatchObject({
message: "Cannot establish inherited Linux limit: Max address space",
});
expect(physical.owner.handle.fd).toBeGreaterThanOrEqual(0);
if (scoped)
await expect(scope.retryCleanup()).rejects.toMatchObject({
cleanupLeases: [failure.cleanupLease],
});
else
await expect(failure.cleanupLease.release()).rejects.toThrow(
"Kernel descriptor close denied",
);
kernel.denied = false;
if (scoped) await scope.retryCleanup();
else await failure.cleanupLease.release();
expect(failure.cleanupLease.released).toBe(true);
expect(physical.owner.handle.fd).toBe(-1);
expect(scope.hasPending).toBe(false);
expect(failure.fileCleanupFailure).toBe(physical);
},
);
it("readmits analysis and resolves the original cleanup error after kernel descriptor recovery", async () => {
const directory = await createTestTempDirectory("rea-kernel-admission-");
kernel.path = join(directory, "limits");
kernel.denied = true;
const resources = new ArtifactAnalysisResources(
"kernel-test",
"read-limits",
readStableArtifact,
readLinuxResourceLimits,
{ message: "busy", details: {} },
);
onTestFinished(async () => {
kernel.denied = false;
await resources.close();
kernel.path = "";
});
await writeFile(kernel.path, report("unlimited unlimited"));
const read = () =>
resources.run(async () => ok(await resources.readLimits(configured)));
const first = await read();
if (first.ok) throw new Error("Expected denied descriptor cleanup");
expect(first.error.cleanupIncomplete).toBe(true);
const opened = kernel.opens;
const blocked = await read();
expect(blocked.ok).toBe(false);
expect(kernel.opens).toBe(opened);
kernel.denied = false;
const recovered = await read();
expect(recovered).toEqual(ok(configured));
expect(kernel.opens).toBe(opened + 1);
expect(first.error.cleanupIncomplete).toBe(false);
if (!blocked.ok) expect(blocked.error.cleanupIncomplete).toBe(false);
});
+9 -26
View File
@@ -1,7 +1,6 @@
import { open } from "node:fs/promises";
import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js";
import { withArtifactFileHandle } from "../artifacts/ArtifactFileOperation.js";
import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js";
/** Per-process ceilings inherited by an owned Linux producer. */
export interface LinuxResourceLimits {
@@ -70,36 +69,20 @@ export const readLinuxResourceLimits = async (
resourceScope?: ArtifactResourceScope,
): Promise<LinuxResourceLimits> => {
const file = await open("/proc/self/limits", "r");
const owner = new OwnedFileHandle(file);
let failure: unknown;
try {
return withArtifactFileHandle(
"/proc/self/limits",
file,
resourceScope,
undefined,
async (handle) => {
const buffer = Buffer.alloc(16 * 1024);
const { bytesRead } = await file.read(buffer, 0, buffer.length, 0);
const { bytesRead } = await handle.read(buffer, 0, buffer.length, 0);
if (bytesRead === buffer.length)
throw new Error("Inherited Linux limits exceed the report budget.");
return clampLinuxResourceLimits(
configured,
buffer.subarray(0, bytesRead).toString("utf8"),
);
} catch (cause: unknown) {
failure = cause;
throw cause;
} finally {
if (resourceScope === undefined) await file.close();
else {
const cleaned = await resourceScope.release({
kind: "file-handle",
resource: "/proc/self/limits",
handle: owner,
});
if (cleaned.kind === "failed")
throw ArtifactReaderFailure.withCleanup(
failure ?? cleaned.cause,
ArtifactReaderFailure.cleanupObservation(
cleaned.cause,
"/proc/self/limits",
),
},
);
}
}
};