fix(process): avoid cold ownership helper deadline exhaustion (#1139)
* fix(process): avoid cold ownership helper deadline exhaustion Fixes #1137. * test(process): fix cold ownership test deadlines and cleanup * fix(process): isolate Swift cache overrides and retain diagnostics * test(electron): assert blank path rejection without trim assumptions --------- Co-authored-by: Chris <Chis@RICH-13918M.local> Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
This commit is contained in:
parent
4034883e98
commit
65a809a0da
|
|
@ -26,6 +26,7 @@ import {
|
|||
type OwnedProviderProcessSpawnOptions,
|
||||
type SpawnedOwnedProviderProcess,
|
||||
} from "../../process/ProviderProcess.js";
|
||||
import { prepareProcessOwnershipInspection } from "../../process/ProcessOwnershipObservation.js";
|
||||
|
||||
const OPERATION = "trace_web_source_location";
|
||||
const PROVIDER = "source-map-decoder";
|
||||
|
|
@ -43,6 +44,8 @@ export interface SourceMapDecoderDependencies {
|
|||
readonly launcher?: SourceMapCodecLauncher;
|
||||
readonly createRuntime?: () => Promise<SourceMapCodecRuntime>;
|
||||
readonly environment?: Readonly<NodeJS.ProcessEnv>;
|
||||
/** Prepare host ownership inspection outside the codec execution deadline. */
|
||||
readonly prepareOwnership?: (signal?: AbortSignal) => Promise<void>;
|
||||
}
|
||||
|
||||
/** Run a trusted codec command and release its process/root before returning output. */
|
||||
|
|
@ -52,6 +55,24 @@ export const runSourceMapCommand = async (
|
|||
options: ExecutionOptions | undefined,
|
||||
dependencies: SourceMapDecoderDependencies,
|
||||
): Promise<Result<string, AnalysisError>> => {
|
||||
try {
|
||||
await (dependencies.prepareOwnership ?? prepareProcessOwnershipInspection)(
|
||||
options?.signal,
|
||||
);
|
||||
} catch (cause: unknown) {
|
||||
return err(
|
||||
options?.signal?.aborted === true
|
||||
? new AnalysisCancelledError(OPERATION)
|
||||
: new ProviderAdapterError(PROVIDER, OPERATION, {
|
||||
cause,
|
||||
diagnostics: {
|
||||
map_path: path,
|
||||
error_message: message(cause),
|
||||
phase: "process_ownership_preparation",
|
||||
},
|
||||
}),
|
||||
);
|
||||
}
|
||||
const deadline = new ProviderStartupDeadline(
|
||||
WEB_SOURCE_MAP_LIMITS.decodeTimeoutMs,
|
||||
options?.signal,
|
||||
|
|
|
|||
|
|
@ -149,21 +149,18 @@ export const createDarwinProcessRunTokenReader = (
|
|||
exitCleanupInstalled = true;
|
||||
if (signal.aborted || closed) throw abortReason(signal);
|
||||
const output = join(operationRoot, "reader");
|
||||
const compilerEnvironment = { ...process.env };
|
||||
// Use Swift's reusable default cache for REA's internal helper compilation.
|
||||
delete compilerEnvironment.CLANG_MODULE_CACHE_PATH;
|
||||
try {
|
||||
await execFileOutput(
|
||||
options.xcrun ?? "/usr/bin/xcrun",
|
||||
[
|
||||
"swiftc",
|
||||
"-module-cache-path",
|
||||
join(operationRoot, "modules"),
|
||||
...sourceFiles,
|
||||
"-o",
|
||||
output,
|
||||
],
|
||||
["swiftc", ...sourceFiles, "-o", output],
|
||||
{
|
||||
timeout: 60_000,
|
||||
maxBuffer: 1024 * 1024,
|
||||
signal,
|
||||
env: compilerEnvironment,
|
||||
},
|
||||
);
|
||||
} catch (cause: unknown) {
|
||||
|
|
@ -173,7 +170,9 @@ export const createDarwinProcessRunTokenReader = (
|
|||
? String(cause.code)
|
||||
: "unknown";
|
||||
throw new DarwinProcessOwnershipInspectionError(
|
||||
`macOS process ownership inspection requires the Apple Swift compiler via xcrun (compiler result: ${code})`,
|
||||
code === "ENOENT"
|
||||
? `macOS process ownership inspection requires the Apple Swift compiler via xcrun (compiler result: ${code})`
|
||||
: `macOS process ownership helper compilation failed via xcrun (compiler result: ${code}): ${cause instanceof Error ? cause.message : String(cause)}`,
|
||||
{ cause },
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ cliTest.for(["", " \t "])(
|
|||
details: {
|
||||
operation: "capture_electron_scenario",
|
||||
issues: expect.arrayContaining([
|
||||
{ path: ["application_root"], reason: "out_of_range", minimum: 1 },
|
||||
expect.objectContaining({ path: ["application_root"] }),
|
||||
]),
|
||||
},
|
||||
});
|
||||
|
|
|
|||
|
|
@ -36,8 +36,9 @@ cliTest(
|
|||
environment: {
|
||||
REA_LOG_LEVEL: "silent",
|
||||
REA_BROWSER_EXECUTABLE: "relative-unconfigured-browser",
|
||||
CLANG_MODULE_CACHE_PATH: map,
|
||||
},
|
||||
timeoutMs: 15000,
|
||||
timeoutMs: 45000,
|
||||
});
|
||||
expect(response.exitCode).toBe(0);
|
||||
expect(response.json).toMatchObject({
|
||||
|
|
@ -56,7 +57,11 @@ cliTest(
|
|||
fileURLToPath(new URL("../../scripts/rea.mjs", import.meta.url)),
|
||||
"mcp",
|
||||
],
|
||||
env: { ...getDefaultEnvironment(), REA_LOG_LEVEL: "silent" },
|
||||
env: {
|
||||
...getDefaultEnvironment(),
|
||||
REA_LOG_LEVEL: "silent",
|
||||
CLANG_MODULE_CACHE_PATH: map,
|
||||
},
|
||||
stderr: "pipe",
|
||||
});
|
||||
const client = new Client({
|
||||
|
|
@ -96,7 +101,7 @@ cliTest(
|
|||
"--json",
|
||||
],
|
||||
environment: { REA_LOG_LEVEL: "silent" },
|
||||
timeoutMs: 15000,
|
||||
timeoutMs: 45000,
|
||||
});
|
||||
expect(invalid.exitCode).toBe(1);
|
||||
expect(invalid.json).toMatchObject({
|
||||
|
|
@ -104,4 +109,6 @@ cliTest(
|
|||
details: { issues: [{ path: ["generated_position"] }] },
|
||||
});
|
||||
},
|
||||
// Allow cold CLI and MCP ownership preparation across the full parity sequence.
|
||||
180_000,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { fileURLToPath } from "node:url";
|
||||
import type { SourceMapDecoderDependencies } from "../../../src/javascript/sourceMaps/SourceMapDecoder.js";
|
||||
import { access } from "node:fs/promises";
|
||||
import { expect, it } from "vitest";
|
||||
import { expect, it, vi } from "vitest";
|
||||
import { SourceMapDecoder } from "../../../src/javascript/sourceMaps/SourceMapDecoder.js";
|
||||
import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js";
|
||||
import { spawnOwnedProviderProcess } from "../../../src/process/ProviderProcess.js";
|
||||
|
|
@ -41,6 +41,32 @@ const input = {
|
|||
position: webSourceLocationArgs.generated_position,
|
||||
};
|
||||
|
||||
it("starts the codec deadline after cold ownership preparation", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
let prepared = false;
|
||||
const responsePromise = realDecoder({
|
||||
prepareOwnership: async (signal) => {
|
||||
await vi.advanceTimersByTimeAsync(20_001);
|
||||
expect(signal?.aborted).not.toBe(true);
|
||||
prepared = true;
|
||||
},
|
||||
launcher: (options) => {
|
||||
expect(prepared).toBe(true);
|
||||
expect(options.signal?.aborted).not.toBe(true);
|
||||
return Promise.reject(new Error("observed launch after preparation"));
|
||||
},
|
||||
}).trace(input);
|
||||
const response = await responsePromise;
|
||||
if (response.ok) throw new Error("expected launch failure");
|
||||
expect(response.error).toMatchObject({
|
||||
diagnostics: { error_message: "observed launch after preparation" },
|
||||
});
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("runs the real pinned codec with an independent heap and removes its private request directory", async () => {
|
||||
let path: string | undefined;
|
||||
const environment = {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { execFile, spawn, type ChildProcess } from "node:child_process";
|
|||
import {
|
||||
access,
|
||||
chmod,
|
||||
mkdir,
|
||||
mkdtemp,
|
||||
readFile,
|
||||
rm,
|
||||
|
|
@ -14,6 +15,7 @@ import { fileURLToPath } from "node:url";
|
|||
import { promisify } from "node:util";
|
||||
import { _electron as electron } from "playwright-core";
|
||||
import { expect, it, vi } from "vitest";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
|
||||
import {
|
||||
createSystemProcessOwnershipHost,
|
||||
|
|
@ -41,10 +43,10 @@ const state = fs.existsSync(statePath)
|
|||
? JSON.parse(fs.readFileSync(statePath, "utf8"))
|
||||
: { calls: 0 };
|
||||
state.calls += 1;
|
||||
const moduleIndex = process.argv.indexOf("-module-cache-path");
|
||||
state.root = path.dirname(process.argv[moduleIndex + 1]);
|
||||
const outputIndex = process.argv.indexOf("-o");
|
||||
state.output = process.argv[outputIndex + 1];
|
||||
state.root = path.dirname(state.output);
|
||||
state.arguments = process.argv.slice(2);
|
||||
state.releasePath = releasePath;
|
||||
fs.writeFileSync(statePath, JSON.stringify(state));
|
||||
if (state.calls === 1) {
|
||||
|
|
@ -106,6 +108,10 @@ const waitForCompileState = async (statePath: string) => {
|
|||
root: parsed.root,
|
||||
output: parsed.output,
|
||||
releasePath: parsed.releasePath,
|
||||
arguments:
|
||||
"arguments" in parsed && Array.isArray(parsed.arguments)
|
||||
? parsed.arguments
|
||||
: [],
|
||||
};
|
||||
} catch {
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
|
|
@ -161,6 +167,87 @@ it("reports an actionable missing Swift compiler without installing it", async (
|
|||
}
|
||||
});
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"preserves compiler diagnostics when Swift preparation fails",
|
||||
async () => {
|
||||
const directory = await createTestTempDirectory(
|
||||
"rea-process-token-compiler-failure-test-",
|
||||
);
|
||||
const executable = join(directory, "failing-xcrun");
|
||||
await writeFile(
|
||||
executable,
|
||||
'#!/usr/bin/env node\nprocess.stderr.write("module cache is not writable\\n"); process.exitCode = 1;\n',
|
||||
);
|
||||
await chmod(executable, 0o755);
|
||||
const reader = createDarwinProcessRunTokenReader({ xcrun: executable });
|
||||
try {
|
||||
await expect(reader.prepare()).rejects.toThrow(
|
||||
/compilation failed.*module cache is not writable/su,
|
||||
);
|
||||
} finally {
|
||||
await reader.close();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it
|
||||
.skipIf(!onDarwin || process.getuid?.() === 0)
|
||||
.each(["file", "read-only directory"])(
|
||||
"ignores unusable inherited Swift module cache: %s",
|
||||
async (kind) => {
|
||||
const directory = await createTestTempDirectory(
|
||||
"rea-process-token-inherited-cache-test-",
|
||||
);
|
||||
const cacheParent = join(directory, "cache-parent");
|
||||
if (kind === "file") await writeFile(cacheParent, "not a directory");
|
||||
else {
|
||||
await mkdir(cacheParent);
|
||||
await chmod(cacheParent, 0o500);
|
||||
}
|
||||
const cachePath = join(cacheParent, "modules");
|
||||
vi.stubEnv("CLANG_MODULE_CACHE_PATH", cachePath);
|
||||
const reader = createDarwinProcessRunTokenReader();
|
||||
try {
|
||||
const executable = await reader.prepare();
|
||||
const { stdout } = await execFileOutput(
|
||||
executable,
|
||||
["--identities", String(process.pid)],
|
||||
{ timeout: 15_000 },
|
||||
);
|
||||
expect(JSON.parse(stdout)).toMatchObject({
|
||||
results: [{ pid: process.pid, state: "readable" }],
|
||||
});
|
||||
expect(process.env.CLANG_MODULE_CACHE_PATH).toBe(cachePath);
|
||||
} finally {
|
||||
vi.unstubAllEnvs();
|
||||
if (kind === "read-only directory") await chmod(cacheParent, 0o700);
|
||||
await reader.close();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"uses the compiler-managed module cache instead of rebuilding system modules per REA process",
|
||||
async () => {
|
||||
const directory = await createTestTempDirectory(
|
||||
"rea-process-token-module-cache-test-",
|
||||
);
|
||||
const fakeCompiler = await writeBlockingCompiler(directory);
|
||||
const reader = createDarwinProcessRunTokenReader({
|
||||
xcrun: fakeCompiler.executable,
|
||||
});
|
||||
const preparation = reader.prepare();
|
||||
try {
|
||||
const state = await waitForCompileState(fakeCompiler.statePath);
|
||||
expect(state.arguments).not.toContain("-module-cache-path");
|
||||
await writeFile(fakeCompiler.releasePath, "release");
|
||||
await preparation;
|
||||
} finally {
|
||||
await Promise.all([reader.close(), Promise.allSettled([preparation])]);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"rethrows compile cancellation, removes its temporary root, and permits retry",
|
||||
async () => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue