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 <[email protected]> Co-authored-by: morluto <[email protected]>
This commit is contained in:
6 files changed
+156
-16
No files matched your search
@@ -26,6 +26,7 @@ import {
|
|||||||
type OwnedProviderProcessSpawnOptions,
|
type OwnedProviderProcessSpawnOptions,
|
||||||
type SpawnedOwnedProviderProcess,
|
type SpawnedOwnedProviderProcess,
|
||||||
} from "../../process/ProviderProcess.js";
|
} from "../../process/ProviderProcess.js";
|
||||||
|
import { prepareProcessOwnershipInspection } from "../../process/ProcessOwnershipObservation.js";
|
||||||
|
|
||||||
const OPERATION = "trace_web_source_location";
|
const OPERATION = "trace_web_source_location";
|
||||||
const PROVIDER = "source-map-decoder";
|
const PROVIDER = "source-map-decoder";
|
||||||
@@ -43,6 +44,8 @@ export interface SourceMapDecoderDependencies {
|
|||||||
readonly launcher?: SourceMapCodecLauncher;
|
readonly launcher?: SourceMapCodecLauncher;
|
||||||
readonly createRuntime?: () => Promise<SourceMapCodecRuntime>;
|
readonly createRuntime?: () => Promise<SourceMapCodecRuntime>;
|
||||||
readonly environment?: Readonly<NodeJS.ProcessEnv>;
|
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. */
|
/** Run a trusted codec command and release its process/root before returning output. */
|
||||||
@@ -52,6 +55,24 @@ export const runSourceMapCommand = async (
|
|||||||
options: ExecutionOptions | undefined,
|
options: ExecutionOptions | undefined,
|
||||||
dependencies: SourceMapDecoderDependencies,
|
dependencies: SourceMapDecoderDependencies,
|
||||||
): Promise<Result<string, AnalysisError>> => {
|
): 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(
|
const deadline = new ProviderStartupDeadline(
|
||||||
WEB_SOURCE_MAP_LIMITS.decodeTimeoutMs,
|
WEB_SOURCE_MAP_LIMITS.decodeTimeoutMs,
|
||||||
options?.signal,
|
options?.signal,
|
||||||
|
|||||||
@@ -149,21 +149,18 @@ export const createDarwinProcessRunTokenReader = (
|
|||||||
exitCleanupInstalled = true;
|
exitCleanupInstalled = true;
|
||||||
if (signal.aborted || closed) throw abortReason(signal);
|
if (signal.aborted || closed) throw abortReason(signal);
|
||||||
const output = join(operationRoot, "reader");
|
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 {
|
try {
|
||||||
await execFileOutput(
|
await execFileOutput(
|
||||||
options.xcrun ?? "/usr/bin/xcrun",
|
options.xcrun ?? "/usr/bin/xcrun",
|
||||||
[
|
["swiftc", ...sourceFiles, "-o", output],
|
||||||
"swiftc",
|
|
||||||
"-module-cache-path",
|
|
||||||
join(operationRoot, "modules"),
|
|
||||||
...sourceFiles,
|
|
||||||
"-o",
|
|
||||||
output,
|
|
||||||
],
|
|
||||||
{
|
{
|
||||||
timeout: 60_000,
|
timeout: 60_000,
|
||||||
maxBuffer: 1024 * 1024,
|
maxBuffer: 1024 * 1024,
|
||||||
signal,
|
signal,
|
||||||
|
env: compilerEnvironment,
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
} catch (cause: unknown) {
|
} catch (cause: unknown) {
|
||||||
@@ -173,7 +170,9 @@ export const createDarwinProcessRunTokenReader = (
|
|||||||
? String(cause.code)
|
? String(cause.code)
|
||||||
: "unknown";
|
: "unknown";
|
||||||
throw new DarwinProcessOwnershipInspectionError(
|
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 },
|
{ cause },
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ cliTest.for(["", " \t "])(
|
|||||||
details: {
|
details: {
|
||||||
operation: "capture_electron_scenario",
|
operation: "capture_electron_scenario",
|
||||||
issues: expect.arrayContaining([
|
issues: expect.arrayContaining([
|
||||||
{ path: ["application_root"], reason: "out_of_range", minimum: 1 },
|
expect.objectContaining({ path: ["application_root"] }),
|
||||||
]),
|
]),
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -36,8 +36,9 @@ cliTest(
|
|||||||
environment: {
|
environment: {
|
||||||
REA_LOG_LEVEL: "silent",
|
REA_LOG_LEVEL: "silent",
|
||||||
REA_BROWSER_EXECUTABLE: "relative-unconfigured-browser",
|
REA_BROWSER_EXECUTABLE: "relative-unconfigured-browser",
|
||||||
|
CLANG_MODULE_CACHE_PATH: map,
|
||||||
},
|
},
|
||||||
timeoutMs: 15000,
|
timeoutMs: 45000,
|
||||||
});
|
});
|
||||||
expect(response.exitCode).toBe(0);
|
expect(response.exitCode).toBe(0);
|
||||||
expect(response.json).toMatchObject({
|
expect(response.json).toMatchObject({
|
||||||
@@ -56,7 +57,11 @@ cliTest(
|
|||||||
fileURLToPath(new URL("../../scripts/rea.mjs", import.meta.url)),
|
fileURLToPath(new URL("../../scripts/rea.mjs", import.meta.url)),
|
||||||
"mcp",
|
"mcp",
|
||||||
],
|
],
|
||||||
env: { ...getDefaultEnvironment(), REA_LOG_LEVEL: "silent" },
|
env: {
|
||||||
|
...getDefaultEnvironment(),
|
||||||
|
REA_LOG_LEVEL: "silent",
|
||||||
|
CLANG_MODULE_CACHE_PATH: map,
|
||||||
|
},
|
||||||
stderr: "pipe",
|
stderr: "pipe",
|
||||||
});
|
});
|
||||||
const client = new Client({
|
const client = new Client({
|
||||||
@@ -96,7 +101,7 @@ cliTest(
|
|||||||
"--json",
|
"--json",
|
||||||
],
|
],
|
||||||
environment: { REA_LOG_LEVEL: "silent" },
|
environment: { REA_LOG_LEVEL: "silent" },
|
||||||
timeoutMs: 15000,
|
timeoutMs: 45000,
|
||||||
});
|
});
|
||||||
expect(invalid.exitCode).toBe(1);
|
expect(invalid.exitCode).toBe(1);
|
||||||
expect(invalid.json).toMatchObject({
|
expect(invalid.json).toMatchObject({
|
||||||
@@ -104,4 +109,6 @@ cliTest(
|
|||||||
details: { issues: [{ path: ["generated_position"] }] },
|
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 { fileURLToPath } from "node:url";
|
||||||
import type { SourceMapDecoderDependencies } from "../../../src/javascript/sourceMaps/SourceMapDecoder.js";
|
import type { SourceMapDecoderDependencies } from "../../../src/javascript/sourceMaps/SourceMapDecoder.js";
|
||||||
import { access } from "node:fs/promises";
|
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 { SourceMapDecoder } from "../../../src/javascript/sourceMaps/SourceMapDecoder.js";
|
||||||
import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js";
|
import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js";
|
||||||
import { spawnOwnedProviderProcess } from "../../../src/process/ProviderProcess.js";
|
import { spawnOwnedProviderProcess } from "../../../src/process/ProviderProcess.js";
|
||||||
@@ -41,6 +41,32 @@ const input = {
|
|||||||
position: webSourceLocationArgs.generated_position,
|
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 () => {
|
it("runs the real pinned codec with an independent heap and removes its private request directory", async () => {
|
||||||
let path: string | undefined;
|
let path: string | undefined;
|
||||||
const environment = {
|
const environment = {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { execFile, spawn, type ChildProcess } from "node:child_process";
|
|||||||
import {
|
import {
|
||||||
access,
|
access,
|
||||||
chmod,
|
chmod,
|
||||||
|
mkdir,
|
||||||
mkdtemp,
|
mkdtemp,
|
||||||
readFile,
|
readFile,
|
||||||
rm,
|
rm,
|
||||||
@@ -14,6 +15,7 @@ import { fileURLToPath } from "node:url";
|
|||||||
import { promisify } from "node:util";
|
import { promisify } from "node:util";
|
||||||
import { _electron as electron } from "playwright-core";
|
import { _electron as electron } from "playwright-core";
|
||||||
import { expect, it, vi } from "vitest";
|
import { expect, it, vi } from "vitest";
|
||||||
|
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||||
|
|
||||||
import {
|
import {
|
||||||
createSystemProcessOwnershipHost,
|
createSystemProcessOwnershipHost,
|
||||||
@@ -41,10 +43,10 @@ const state = fs.existsSync(statePath)
|
|||||||
? JSON.parse(fs.readFileSync(statePath, "utf8"))
|
? JSON.parse(fs.readFileSync(statePath, "utf8"))
|
||||||
: { calls: 0 };
|
: { calls: 0 };
|
||||||
state.calls += 1;
|
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");
|
const outputIndex = process.argv.indexOf("-o");
|
||||||
state.output = process.argv[outputIndex + 1];
|
state.output = process.argv[outputIndex + 1];
|
||||||
|
state.root = path.dirname(state.output);
|
||||||
|
state.arguments = process.argv.slice(2);
|
||||||
state.releasePath = releasePath;
|
state.releasePath = releasePath;
|
||||||
fs.writeFileSync(statePath, JSON.stringify(state));
|
fs.writeFileSync(statePath, JSON.stringify(state));
|
||||||
if (state.calls === 1) {
|
if (state.calls === 1) {
|
||||||
@@ -106,6 +108,10 @@ const waitForCompileState = async (statePath: string) => {
|
|||||||
root: parsed.root,
|
root: parsed.root,
|
||||||
output: parsed.output,
|
output: parsed.output,
|
||||||
releasePath: parsed.releasePath,
|
releasePath: parsed.releasePath,
|
||||||
|
arguments:
|
||||||
|
"arguments" in parsed && Array.isArray(parsed.arguments)
|
||||||
|
? parsed.arguments
|
||||||
|
: [],
|
||||||
};
|
};
|
||||||
} catch {
|
} catch {
|
||||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
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")(
|
it.skipIf(process.platform === "win32")(
|
||||||
"rethrows compile cancellation, removes its temporary root, and permits retry",
|
"rethrows compile cancellation, removes its temporary root, and permits retry",
|
||||||
async () => {
|
async () => {
|
||||||
|
|||||||
Reference in new issue
Block a user