mirror of
https://github.com/morluto/rea.git
synced 2026-10-19 04:19:54 -05:00
fix(windows): retry owned runtime cleanup on sharing conflicts
Bound retries of native close conflicts at the retained runtime owner. Preserve exact failures and later retry authority after exhaustion. Attempt all verifier cleanup while retaining primary failures; extend held-file Windows verification. Refs #4136, #4185 Co-authored-by: DeryFerd <deryferdikao125@gmail.com>
This commit is contained in:
1 parent
8d8257928b
commit
0679a7972b
7 files changed
+320
-41
No files matched your search
@@ -111,9 +111,14 @@ automatically. Alternatively, build it using the Linux/MinGW lane above.
|
||||
- Cleanup walks locked directory objects, removes files by handle, and unlinks
|
||||
reparse entries without traversing their targets. Failed removal retains the
|
||||
root and ancestor leases for retry; a partially removed runtime accepts only
|
||||
cleanup, and its registry entry remains until removal succeeds. The owned root
|
||||
handle includes attribute-write access so cleanup can clear a read-only marker,
|
||||
matching the child cleanup handles. Normal close, cancellation,
|
||||
cleanup, and its registry entry remains until removal succeeds. The runtime
|
||||
owner retries only native close failures with Win32 sharing/lock codes 32/33,
|
||||
for at most ten attempts with a 100 ms delay between attempts. Other native
|
||||
errors are returned immediately; exhausting the retry budget preserves the
|
||||
runtime owner for a later explicit close. These cleanup retries do not use an
|
||||
analysis cancellation signal and schedule up to 900 ms of backoff, in
|
||||
addition to native cleanup time. The owned root handle includes attribute-write access so cleanup can clear a
|
||||
read-only marker, matching the child cleanup handles. Normal close, cancellation,
|
||||
startup failure, and timeout release provider resources. Abrupt owner death
|
||||
kills the owned job. The Android provider additionally uses a detached helper
|
||||
that owns its private runtime directory and cleans it when the owner pipe
|
||||
@@ -133,7 +138,9 @@ text do not establish a safe retry. Analysis errors propagate the producer's
|
||||
retry disposition through JavaScript, source-map and WABT workflows; transient
|
||||
failures report `category: unavailable` and retain the original reason.
|
||||
Incomplete cleanup blocks an operation retry until its owned resources settle.
|
||||
REA adds no automatic retry loop or new analysis cache for this behavior.
|
||||
The close-stage retry is separate from allocation and analysis retries: it
|
||||
repeats only the existing owned cleanup operation, and does not retry provider
|
||||
work or create another runtime.
|
||||
|
||||
Ordinary drive-absolute paths accept backslashes, forward slashes, or mixed
|
||||
separators. File identity retains the original requested spelling. Separator
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
import { join } from "node:path";
|
||||
import { mkdir } from "node:fs/promises";
|
||||
import { PrivateRuntimeRoot } from "../dist/process/PrivateRuntimeRoot.js";
|
||||
import { closeVerifierResources } from "./lib/verifier-cleanup.mjs";
|
||||
import { exec, execNpm } from "./lib/verify-package-core.mjs";
|
||||
import { WABT_WORKFLOW_TIMEOUT_MS } from "./lib/wabt-verifier-timeouts.mjs";
|
||||
import { verifyPackagePack } from "./verify-package-pack.mjs";
|
||||
@@ -9,6 +10,7 @@ import { verifyPackagePack } from "./verify-package-pack.mjs";
|
||||
const root = await PrivateRuntimeRoot.create({
|
||||
prefix: "rea-wabt-package-verifier-",
|
||||
});
|
||||
let primaryError;
|
||||
try {
|
||||
const { tarball } = await verifyPackagePack({
|
||||
root: process.cwd(),
|
||||
@@ -56,6 +58,9 @@ try {
|
||||
},
|
||||
);
|
||||
process.stdout.write(verified.stdout);
|
||||
} catch (cause) {
|
||||
primaryError = cause;
|
||||
throw cause;
|
||||
} finally {
|
||||
await root.close();
|
||||
await closeVerifierResources([() => root.close()], primaryError);
|
||||
}
|
||||
@@ -12,6 +12,7 @@ import { StdioClientTransport } from "@modelcontextprotocol/client/stdio";
|
||||
import { Ajv2020 } from "ajv/dist/2020.js";
|
||||
import { PrivateRuntimeRoot } from "../dist/process/PrivateRuntimeRoot.js";
|
||||
import { parseEvidence } from "../dist/domain/evidence.js";
|
||||
import { closeVerifierResources } from "./lib/verifier-cleanup.mjs";
|
||||
import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs";
|
||||
|
||||
if (process.platform !== "linux")
|
||||
@@ -106,6 +107,7 @@ const expectedAddressSpace = Math.min(
|
||||
.map((value) => (value === "unlimited" ? Infinity : Number(value))),
|
||||
);
|
||||
|
||||
let primaryError;
|
||||
try {
|
||||
await writeFile(path, source);
|
||||
await writeFile(bluetooth, bluetoothSource);
|
||||
@@ -463,10 +465,14 @@ try {
|
||||
"network with spaces.pcap",
|
||||
"paged.pcap",
|
||||
]);
|
||||
} catch (cause) {
|
||||
primaryError = cause;
|
||||
throw cause;
|
||||
} finally {
|
||||
await client.close();
|
||||
await transport.close();
|
||||
await root.close();
|
||||
await closeVerifierResources(
|
||||
[() => client.close(), () => transport.close(), () => root.close()],
|
||||
primaryError,
|
||||
);
|
||||
}
|
||||
console.log(
|
||||
JSON.stringify(
|
||||
|
||||
@@ -12,6 +12,7 @@ import { StdioClientTransport } from "@modelcontextprotocol/client/stdio";
|
||||
import { Ajv2020 } from "ajv/dist/2020.js";
|
||||
import { PrivateRuntimeRoot } from "../dist/process/PrivateRuntimeRoot.js";
|
||||
import { parseEvidence } from "../dist/domain/evidence.js";
|
||||
import { closeVerifierResources } from "./lib/verifier-cleanup.mjs";
|
||||
import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs";
|
||||
|
||||
if (process.platform !== "linux")
|
||||
@@ -83,6 +84,7 @@ const expectedAddressSpace = Math.min(
|
||||
.map((value) => (value === "unlimited" ? Infinity : Number(value))),
|
||||
);
|
||||
|
||||
let primaryError;
|
||||
try {
|
||||
await writeFile(path, source);
|
||||
await client.connect(transport);
|
||||
@@ -383,10 +385,14 @@ try {
|
||||
"paged.vcd",
|
||||
"recording with spaces.vcd",
|
||||
]);
|
||||
} catch (cause) {
|
||||
primaryError = cause;
|
||||
throw cause;
|
||||
} finally {
|
||||
await client.close();
|
||||
await transport.close();
|
||||
await root.close();
|
||||
await closeVerifierResources(
|
||||
[() => client.close(), () => transport.close(), () => root.close()],
|
||||
primaryError,
|
||||
);
|
||||
}
|
||||
console.log(
|
||||
JSON.stringify(
|
||||
|
||||
@@ -23,6 +23,7 @@ import { pathToFileURL } from "node:url";
|
||||
import { verifyWindowsProtocolInput } from "./lib/windows-protocol-input.mjs";
|
||||
import { verifyWindowsJobMemoryLimit } from "./lib/windows-job-memory-limit.mjs";
|
||||
import { verifyVolumeRootAppendLease } from "./lib/windows-volume-root-append-lease.mjs";
|
||||
import { closeVerifierResources } from "./lib/verifier-cleanup.mjs";
|
||||
|
||||
if (process.platform !== "win32")
|
||||
throw new Error(
|
||||
@@ -66,6 +67,49 @@ const exists = (path) =>
|
||||
() => true,
|
||||
() => false,
|
||||
);
|
||||
const startDeleteSharingDeniedReader = async (path) => {
|
||||
const child = spawn(
|
||||
"powershell.exe",
|
||||
[
|
||||
"-NoProfile",
|
||||
"-NonInteractive",
|
||||
"-Command",
|
||||
"$ErrorActionPreference = 'Stop'; " +
|
||||
"$stream = [IO.File]::Open($env:REA_CLEANUP_HELD_PATH, [IO.FileMode]::Open, [IO.FileAccess]::Read, [IO.FileShare]::ReadWrite); " +
|
||||
"try { [Console]::WriteLine('ready'); [Console]::In.ReadLine() | Out-Null } " +
|
||||
"finally { $stream.Dispose() }",
|
||||
],
|
||||
{
|
||||
env: { ...process.env, REA_CLEANUP_HELD_PATH: path },
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
},
|
||||
);
|
||||
const closed = once(child, "close");
|
||||
void closed.catch(() => undefined);
|
||||
try {
|
||||
const [ready] = await once(child.stdout, "data", {
|
||||
signal: AbortSignal.timeout(15_000),
|
||||
});
|
||||
assert.equal(String(ready).trim(), "ready");
|
||||
} catch (cause) {
|
||||
try {
|
||||
child.kill();
|
||||
await closed;
|
||||
} catch (cleanupCause) {
|
||||
throw new AggregateError(
|
||||
[cause, cleanupCause],
|
||||
"Reader startup and child cleanup failed",
|
||||
{ cause },
|
||||
);
|
||||
}
|
||||
throw cause;
|
||||
}
|
||||
return { child, closed };
|
||||
};
|
||||
const stopDeleteSharingDeniedReader = async ({ child, closed }) => {
|
||||
child.kill();
|
||||
await closed;
|
||||
};
|
||||
const live = (pid) => {
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
@@ -516,32 +560,14 @@ process.stdout.write(JSON.stringify({executable:basename(process.execPath),abiVe
|
||||
"cleanup-retry-",
|
||||
);
|
||||
const heldPath = join(retryRuntime.observation.path, "snapshot.bin");
|
||||
await retryRuntime.snapshot(source, "snapshot.bin");
|
||||
const holder = spawn(
|
||||
"powershell.exe",
|
||||
[
|
||||
"-NoProfile",
|
||||
"-NonInteractive",
|
||||
"-Command",
|
||||
"$ErrorActionPreference = 'Stop'; " +
|
||||
"$stream = [IO.File]::Open($env:REA_CLEANUP_HELD_PATH, [IO.FileMode]::Open, [IO.FileAccess]::Read, [IO.FileShare]::ReadWrite); " +
|
||||
"try { [Console]::WriteLine('ready'); [Console]::In.ReadLine() | Out-Null } " +
|
||||
"finally { $stream.Dispose() }",
|
||||
],
|
||||
{
|
||||
env: { ...process.env, REA_CLEANUP_HELD_PATH: heldPath },
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
},
|
||||
);
|
||||
const holderClosed = once(holder, "close");
|
||||
void holderClosed.catch(() => undefined);
|
||||
let holder;
|
||||
let primaryError;
|
||||
try {
|
||||
const [ready] = await once(holder.stdout, "data", {
|
||||
signal: AbortSignal.timeout(15_000),
|
||||
});
|
||||
assert.equal(String(ready).trim(), "ready");
|
||||
await retryRuntime.snapshot(source, "snapshot.bin");
|
||||
holder = await startDeleteSharingDeniedReader(heldPath);
|
||||
// The reader denies delete sharing. Cleanup must retain ownership when
|
||||
// its handle-relative open receives ERROR_SHARING_VIOLATION (32).
|
||||
// its handle-relative open receives ERROR_SHARING_VIOLATION (32) through
|
||||
// the bounded automatic retries; a later explicit close must succeed.
|
||||
await assert.rejects(retryRuntime.close(), { win32Code: 32 });
|
||||
assert.equal(
|
||||
windowsPrivateRuntime(retryRuntime.observation.path),
|
||||
@@ -552,8 +578,8 @@ process.stdout.write(JSON.stringify({executable:basename(process.execPath),abiVe
|
||||
() => retryRuntime.writeFile("after-close.txt", "forbidden"),
|
||||
/cleanup is pending/u,
|
||||
);
|
||||
holder.stdin.end("\n");
|
||||
const [exitCode] = await holderClosed;
|
||||
holder.child.stdin.end("\n");
|
||||
const [exitCode] = await holder.closed;
|
||||
assert.equal(exitCode, 0);
|
||||
await retryRuntime.close();
|
||||
await retryRuntime.close();
|
||||
@@ -563,10 +589,62 @@ process.stdout.write(JSON.stringify({executable:basename(process.execPath),abiVe
|
||||
/No native private runtime/u,
|
||||
);
|
||||
report.controls.cleanupFailureRetainsOwnershipForRetry = true;
|
||||
} catch (cause) {
|
||||
primaryError = cause;
|
||||
throw cause;
|
||||
} finally {
|
||||
holder.kill();
|
||||
await holderClosed.catch(() => undefined);
|
||||
await retryRuntime.close();
|
||||
await closeVerifierResources(
|
||||
[
|
||||
...(holder === undefined
|
||||
? []
|
||||
: [() => stopDeleteSharingDeniedReader(holder)]),
|
||||
() => retryRuntime.close(),
|
||||
],
|
||||
primaryError,
|
||||
);
|
||||
}
|
||||
const transientRuntime = WindowsPrivateRuntime.create(
|
||||
workspace,
|
||||
"cleanup-transient-",
|
||||
);
|
||||
const transientHeldPath = join(
|
||||
transientRuntime.observation.path,
|
||||
"snapshot.bin",
|
||||
);
|
||||
let transientHolder;
|
||||
let transientPrimaryError;
|
||||
try {
|
||||
await transientRuntime.snapshot(source, "snapshot.bin");
|
||||
transientHolder = await startDeleteSharingDeniedReader(transientHeldPath);
|
||||
// The first native close is synchronous and must fail while this reader
|
||||
// is held. Releasing it while the owner waits allows its own retry to
|
||||
// complete without the caller issuing another close.
|
||||
const closing = transientRuntime.close();
|
||||
transientHolder.child.stdin.end("\n");
|
||||
const [, [exitCode]] = await Promise.all([
|
||||
closing,
|
||||
transientHolder.closed,
|
||||
]);
|
||||
assert.equal(exitCode, 0);
|
||||
assert.equal(await exists(transientRuntime.observation.path), false);
|
||||
assert.throws(
|
||||
() => windowsPrivateRuntime(transientRuntime.observation.path),
|
||||
/No native private runtime/u,
|
||||
);
|
||||
report.controls.cleanupTransientConflictRetries = true;
|
||||
} catch (cause) {
|
||||
transientPrimaryError = cause;
|
||||
throw cause;
|
||||
} finally {
|
||||
await closeVerifierResources(
|
||||
[
|
||||
...(transientHolder === undefined
|
||||
? []
|
||||
: [() => stopDeleteSharingDeniedReader(transientHolder)]),
|
||||
() => transientRuntime.close(),
|
||||
],
|
||||
transientPrimaryError,
|
||||
);
|
||||
}
|
||||
report.controls = {
|
||||
...report.controls,
|
||||
|
||||
@@ -0,0 +1,151 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
const nativeCall = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("./WindowsNativeLoader.js", async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import("./WindowsNativeLoader.js")>();
|
||||
return {
|
||||
...actual,
|
||||
requireWindowsNativeAuthority: () => ({ call: nativeCall }),
|
||||
};
|
||||
});
|
||||
|
||||
import {
|
||||
WindowsPrivateRuntime,
|
||||
windowsPrivateRuntime,
|
||||
} from "./WindowsPrivateRuntime.js";
|
||||
|
||||
const observation = {
|
||||
requestedPath: "C:\\rea-private",
|
||||
finalPath: "C:\\rea-private",
|
||||
filesystem: "NTFS",
|
||||
volumeSerial: "0123456789abcdef",
|
||||
fileId: "0123456789abcdef0123456789abcdef",
|
||||
size: 0,
|
||||
directory: true,
|
||||
path: "C:\\rea-private",
|
||||
handle: {},
|
||||
privateDacl: true,
|
||||
};
|
||||
|
||||
const nativeFailure = (win32Code: number): Error =>
|
||||
Object.assign(new Error(`native close failed (${win32Code})`), {
|
||||
code: "ERR_REA_WINDOWS_NATIVE",
|
||||
win32Code,
|
||||
});
|
||||
|
||||
describe("Windows private runtime cleanup retry", () => {
|
||||
it("retries a transient sharing conflict through the same owned close", async () => {
|
||||
const conflict = nativeFailure(32);
|
||||
let closes = 0;
|
||||
nativeCall.mockImplementation((operation: string) => {
|
||||
if (operation === "runtime_create") return observation;
|
||||
if (operation === "runtime_close" && closes++ === 0) throw conflict;
|
||||
return null;
|
||||
});
|
||||
const runtime = WindowsPrivateRuntime.create("C:\\", "rea-");
|
||||
|
||||
await runtime.close();
|
||||
|
||||
expect(closes).toBe(2);
|
||||
expect(() => windowsPrivateRuntime(observation.path)).toThrow(
|
||||
/No native private runtime/u,
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["a native access denial", nativeFailure(5)],
|
||||
[
|
||||
"a POSIX-style access denial",
|
||||
Object.assign(new Error("access denied"), {
|
||||
code: "EACCES",
|
||||
win32Code: 32,
|
||||
}),
|
||||
],
|
||||
[
|
||||
"a native error with string code 32",
|
||||
Object.assign(new Error("sharing conflict"), {
|
||||
code: "ERR_REA_WINDOWS_NATIVE",
|
||||
win32Code: "32",
|
||||
}),
|
||||
],
|
||||
[
|
||||
"a native error without a Win32 code",
|
||||
Object.assign(new Error("native failure"), {
|
||||
code: "ERR_REA_WINDOWS_NATIVE",
|
||||
}),
|
||||
],
|
||||
[
|
||||
"a missing-path error carrying a Win32 sharing code",
|
||||
Object.assign(new Error("path disappeared"), {
|
||||
code: "ENOENT",
|
||||
win32Code: 32,
|
||||
}),
|
||||
],
|
||||
[
|
||||
"a nested cleanup detail carrying a Win32 sharing code",
|
||||
Object.assign(new Error("native failure"), {
|
||||
code: "ERR_REA_WINDOWS_NATIVE",
|
||||
cleanup: { win32Code: 32 },
|
||||
}),
|
||||
],
|
||||
] as const)(
|
||||
"does not retry %s and retains the owner for a later close",
|
||||
async (_label, denied) => {
|
||||
let closes = 0;
|
||||
nativeCall.mockImplementation((operation: string) => {
|
||||
if (operation === "runtime_create") return observation;
|
||||
if (operation === "runtime_close" && closes++ === 0) throw denied;
|
||||
return null;
|
||||
});
|
||||
const runtime = WindowsPrivateRuntime.create("C:\\", "rea-");
|
||||
|
||||
await expect(runtime.close()).rejects.toBe(denied);
|
||||
expect(closes).toBe(1);
|
||||
expect(windowsPrivateRuntime(observation.path)).toBe(runtime);
|
||||
await runtime.close();
|
||||
expect(closes).toBe(2);
|
||||
},
|
||||
);
|
||||
|
||||
it("shares one in-flight close across concurrent callers", async () => {
|
||||
let closes = 0;
|
||||
nativeCall.mockImplementation((operation: string) => {
|
||||
if (operation === "runtime_create") return observation;
|
||||
if (operation === "runtime_close" && closes++ === 0)
|
||||
throw nativeFailure(33);
|
||||
return null;
|
||||
});
|
||||
const runtime = WindowsPrivateRuntime.create("C:\\", "rea-");
|
||||
const first = runtime.close();
|
||||
const second = runtime.close();
|
||||
|
||||
expect(second).toBe(first);
|
||||
await Promise.all([first, second]);
|
||||
expect(closes).toBe(2);
|
||||
});
|
||||
|
||||
it("retains the exact final native error and owner when conflicts persist", async () => {
|
||||
const conflict = nativeFailure(32);
|
||||
let closes = 0;
|
||||
nativeCall.mockImplementation((operation: string) => {
|
||||
if (operation === "runtime_create") return observation;
|
||||
if (operation === "runtime_close") {
|
||||
closes++;
|
||||
throw conflict;
|
||||
}
|
||||
return null;
|
||||
});
|
||||
const runtime = WindowsPrivateRuntime.create("C:\\", "rea-");
|
||||
|
||||
await expect(runtime.close()).rejects.toBe(conflict);
|
||||
|
||||
expect(closes).toBe(10);
|
||||
expect(windowsPrivateRuntime(observation.path)).toBe(runtime);
|
||||
nativeCall.mockImplementation((operation: string) =>
|
||||
operation === "runtime_create" ? observation : null,
|
||||
);
|
||||
await runtime.close();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,5 @@
|
||||
import { win32 } from "node:path";
|
||||
import { setTimeout as delay } from "node:timers/promises";
|
||||
|
||||
import { z } from "zod";
|
||||
import { PrivateRuntimeRootUnavailableError } from "../process/PrivateRuntimeRootUnavailableError.js";
|
||||
@@ -31,6 +32,12 @@ const transientRuntimeAdmissionSchema = z.object({
|
||||
win32Code: z.union([z.literal(32), z.literal(33)]),
|
||||
runtimeAllocation: z.literal("not-started"),
|
||||
});
|
||||
const transientRuntimeCleanupSchema = z.object({
|
||||
code: z.literal("ERR_REA_WINDOWS_NATIVE"),
|
||||
win32Code: z.union([z.literal(32), z.literal(33)]),
|
||||
});
|
||||
const RUNTIME_CLEANUP_ATTEMPTS = 10;
|
||||
const RUNTIME_CLEANUP_RETRY_DELAY_MS = 100;
|
||||
|
||||
/** Retain private DACL and path handles until native recursive cleanup settles. */
|
||||
export class WindowsPrivateRuntime {
|
||||
@@ -160,10 +167,29 @@ export class WindowsPrivateRuntime {
|
||||
if (this.#pending !== undefined)
|
||||
this.authority.call("runtime_snapshot_cancel", [this.observation.handle]);
|
||||
await this.#pending?.catch(() => undefined);
|
||||
this.authority.call("runtime_close", [this.observation.handle]);
|
||||
await this.#closeRuntime();
|
||||
this.#closed = true;
|
||||
runtimes.delete(windowsRuntimeIdentityKey(this.observation));
|
||||
}
|
||||
|
||||
async #closeRuntime(): Promise<void> {
|
||||
for (let attempt = 1; ; attempt++) {
|
||||
try {
|
||||
this.authority.call("runtime_close", [this.observation.handle]);
|
||||
return;
|
||||
} catch (cause: unknown) {
|
||||
if (
|
||||
attempt >= RUNTIME_CLEANUP_ATTEMPTS ||
|
||||
!transientRuntimeCleanupSchema.safeParse(cause).success
|
||||
)
|
||||
throw cause;
|
||||
// Native cleanup retains the same owned root and permits only another
|
||||
// close after a failed walk. Retry that owner operation briefly when
|
||||
// Windows reports a sharing/lock conflict; other errors remain final.
|
||||
await delay(RUNTIME_CLEANUP_RETRY_DELAY_MS);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** File id returned by the OS. Path spelling is not part of the lease key. */
|
||||
|
||||
Reference in new issue
Block a user