vrr/openclaw
2
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-11-02 05:27:21 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 34

fix(process): join owned groups through transient EPERM (#158483)

Browse source 
Treat Darwin EPERM as an indeterminate process-group state while preserving
hard signaling errors. Require ESRCH before retiring ownership, cancel the
grace timer on child close, and keep post-close escalation in the bounded
group drain.

Synchronize triage fixtures on the real branch-spawn receipt after maintenance
finishes. Keep the existing descendant assertions and readiness budget, and
abort and join the receipt waiter before cleanup releases fixture state.

Cover transient and persistent EPERM, natural exit before signaling, and a
later hard signaling failure using real owned child processes. All four
regressions fail on the original owner and pass with this repair.

Validation includes failing-before native regressions, repeated process-tree
and triage cases, full test files, typechecks, changed-file type-aware lint,
script lint, and independent review. Full changed checks remain blocked by
pre-existing Telegram test-support unused exports. Full core lint timed out
at its unchanged 900-second limit on both attempts.
This commit is contained in:
Peter Steinberger 2026-09-25 18:43:51 -07:00 • committed by GitHub
parent 06da80236e
commit 478ac90c3e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 475 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

49
.github/actions/ios-signing-keychain/keychain.mjs vendored
View file

@ -130,10 +130,15 @@ export async function runBounded(command, args, options = {}) {
let terminationReason;
let terminationStartedAt;
let terminationError;
let groupExitError;
let closed = false;
let ownedProcessGroupRetired = false;
let forceKillTimer;
const signalOwnedProcess = (signal) => {
if (ownsProcessGroup && ownedProcessGroupRetired) {
return false;
}
try {
if (ownsProcessGroup && child.pid) {
process.kill(-child.pid, signal);
@ -143,22 +148,33 @@ export async function runBounded(command, args, options = {}) {
return true;
} catch (error) {
if (error?.code === "ESRCH") {
ownedProcessGroupRetired = ownsProcessGroup;
return false;
}
terminationError ??= error;
if (ownsProcessGroup && child.pid && error?.code === "EPERM") {
groupExitError ??= error;
} else {
terminationError ??= error;
}
return false;
}
};
const ownedProcessGroupExists = () => {
if (!ownsProcessGroup || !child.pid) {
return false;
const inspectOwnedProcessGroup = () => {
if (!ownsProcessGroup || !child.pid || ownedProcessGroupRetired) {
return "dead";
}
try {
process.kill(-child.pid, 0);
return true;
return "live";
} catch (error) {
if (error?.code === "ESRCH") {
return false;
ownedProcessGroupRetired = true;
return "dead";
}
if (error?.code === "EPERM") {
// Darwin can report EPERM for a zombie-only group until it is reaped.
groupExitError ??= error;
return "indeterminate";
}
throw error;
}
@ -171,7 +187,7 @@ export async function runBounded(command, args, options = {}) {
terminationStartedAt = Date.now();
signalOwnedProcess("SIGTERM");
forceKillTimer = setTimeout(() => {
if (!closed || ownedProcessGroupExists()) {
if (!closed) {
signalOwnedProcess("SIGKILL");
}
}, terminateGraceMs);
@ -196,6 +212,7 @@ export async function runBounded(command, args, options = {}) {
child.once("error", reject);
child.once("close", (code, signal) => {
closed = true;
clearTimeout(forceKillTimer);
resolve({ code, signal });
});
});
@ -211,22 +228,30 @@ export async function runBounded(command, args, options = {}) {
setTimeout(resolve, graceRemaining);
});
}
if (ownedProcessGroupExists()) {
signalOwnedProcess("SIGKILL");
const groupState = inspectOwnedProcessGroup();
if (groupState !== "dead") {
if (groupState === "live") {
signalOwnedProcess("SIGKILL");
}
const joinDeadline = Date.now() + 2_000;
while (ownedProcessGroupExists() && Date.now() < joinDeadline) {
while (inspectOwnedProcessGroup() !== "dead" && Date.now() < joinDeadline) {
await new Promise((resolve) => {
setTimeout(resolve, 10);
});
}
if (ownedProcessGroupExists()) {
throw new Error(`${command} owned process group did not terminate`);
if (inspectOwnedProcessGroup() !== "dead") {
throw new Error(`${command} owned process group did not terminate`, {
cause: terminationError ?? groupExitError,
});
}
}
}
if (terminationError) {
throw terminationError;
}
if (groupExitError && !ownedProcessGroupRetired) {
throw groupExitError;
}
if (terminationReason) {
throw new Error(`${command} ${terminationReason}`);
}

25
src/infra/update-managed-service-triage.test-support.ts
View file

@ -7,11 +7,13 @@ import { createInterface } from "node:readline";
import { DatabaseSync } from "node:sqlite";
import { vi } from "vitest";
import type { FixtureAcquisitionRollback } from "../../test/helpers/fixture-lifetime.js";
import { waitForFixtureFile } from "../../test/helpers/process-wait.js";
import { resolveServiceManagerEnv } from "../daemon/service-process-env.js";
import { resolveSystemdUnitPath } from "../daemon/systemd-service-files.js";
import { buildCliRespawnPlan } from "../entry.respawn.js";
import { getFileLockProcessStartTime, isPidAlive } from "../shared/pid-alive.js";
import { resolveTestNodeExecPath } from "../test-utils/node-process.js";
import { racePromiseWithAbortSignal } from "./abort-signal.js";
import { resolveRuntimeWorkerUrl } from "./runtime-worker-url.js";
import { setSqliteBusyTimeout } from "./sqlite-busy-timeout.js";
import { cleanupTriageBoundary } from "./triage-boundary-cleanup.test-support.js";
@ -73,6 +75,9 @@ async function acquireTriageBoundary(
await fs.mkdir(installRoot, { recursive: true });
await fs.mkdir(candidateRoot, { recursive: true });
const events = path.join(root, "events.jsonl");
const branchReadyPath = path.join(root, "branch-ready");
const fixtureStopping = new AbortController();
let branchReady: Promise<void> | undefined;
const scopeFile = path.join(root, "scope.json");
const primaryFile = path.join(root, "primary.json");
const bin = path.join(root, "bin");
@ -250,6 +255,7 @@ if (${Boolean(maintenance)}) {
event('maintenance-exit',exit);
}
const branch=spawn(process.execPath,['-e',${JSON.stringify(common + "event('descendant', {stateDir:process.env.OPENCLAW_STATE_DIR, workspace:process.env.OPENCLAW_WORKSPACE_DIR, shell:process.env.OPENCLAW_SHELL, compileCache:process.env.NODE_DISABLE_COMPILE_CACHE}); const {spawn}=require('node:child_process'); spawn(process.execPath,['-e','setInterval(()=>{},1000)'],{stdio:'ignore'}); setInterval(()=>{},1000)")}],{detached:true,stdio:'ignore'});
branch.once('spawn',()=>fs.writeFileSync(${JSON.stringify(branchReadyPath)},'ready'));
event('branch',{child:branch.pid});
admission.signal.addEventListener('abort',()=>{event('cancelled');void admission.finish("uncertain");process.exitCode=1;});
await new Promise(resolve=>admission.signal.addEventListener('abort',resolve,{once:true}));
@ -430,6 +436,16 @@ process.stdout.write(JSON.stringify({status:'error',reason:'original failure'})+
helper,
parent,
exit,
waitForBranch: () => {
fixtureStopping.signal.throwIfAborted();
branchReady ??= waitForFixtureFile(
branchReadyPath,
racePromiseWithAbortSignal(exit, fixtureStopping.signal),
).then(() => {
fixtureStopping.signal.throwIfAborted();
});
return branchReady;
},
readEvents,
output: () => output,
stderr: () => stderr,
@ -510,8 +526,10 @@ process.stdout.write(JSON.stringify({status:'error',reason:'original failure'})+
alive: isPidAlive(Number(pid)),
})),
),
cleanup: () =>
cleanupTriageBoundary({
cleanup: async () => {
fixtureStopping.abort();
await branchReady?.catch(() => {});
await cleanupTriageBoundary({
root,
groups,
helper,
@ -521,7 +539,8 @@ process.stdout.write(JSON.stringify({status:'error',reason:'original failure'})+
lines,
readEvents,
databasePath,
}),
});
},
};
}

9
src/infra/update-managed-service-triage.test.ts
View file

@ -27,6 +27,13 @@ async function ready(boundary: Awaited<ReturnType<typeof start>>) {
expect(await boundary.response(), boundary.stderr()).toBe("OPENCLAW_UPDATE_HANDOFF_READY");
}
async function fixing(boundary: Awaited<ReturnType<typeof start>>) {
if (!boundaries.includes(boundary)) {
throw new Error("Branch readiness requires cleanup registered with afterEach");
}
await boundary.waitForBranch();
await expectFixerPlacement(boundary);
}
async function expectFixerPlacement(boundary: Awaited<ReturnType<typeof start>>) {
// One readiness budget covers the fixer and its complete descendant placement.
await vi.waitFor(
async () => {
@ -100,7 +107,7 @@ describe("managed triage attachment cutover (synthetic native boundary)", () =>
});
await ready(boundary);
expect(await boundary.control("commit")).toBe("committed");
await fixing(boundary);
await expectFixerPlacement(boundary);
await vi.waitFor(async () => {
const { pid } = JSON.parse(await fs.readFile(receipt, "utf8"));
controller = { pid, start: getFileLockProcessStartTime(pid) };

8
test/scripts/mobile-release-authority.test.ts
View file

@ -18,6 +18,7 @@ import { applyMobileReleasePlan, planMobileRelease } from "../../scripts/mobile-
import { resolveTestNodeExecPath } from "../../src/test-utils/node-process.js";
import { cleanupTempDirs, makeTempDir, useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";
import { runVitestShutdownCommand } from "../helpers/vitest-shutdown-command.js";
import { registerBoundedSignalTests } from "./mobile-release-process.test-support.js";
const REPOSITORY = "openclaw/openclaw";
const TARGET_REF = "release/2026.9.2-mobile";
@ -4471,6 +4472,8 @@ fi
expect(unsafeCommandCount).toBe(0);
});
registerBoundedSignalTests();
it("bounds owned child process trees", async () => {
const runnerTemp = tempRoots.make("openclaw-ios-keychain-process-runner-");
if (process.platform !== "win32") {
@ -4515,7 +4518,10 @@ try {
}
const processIds = fs.readFileSync(${JSON.stringify(pidFile)}, "utf8").trim().split("\\n").map(Number);
let processGroupAlive = true;
try { process.kill(-processIds[0], 0); } catch { processGroupAlive = false; }
try { process.kill(-processIds[0], 0); } catch (error) {
if (error?.code !== "ESRCH") throw error;
processGroupAlive = false;
}
process.stdout.write(JSON.stringify({ elapsedMs: Date.now() - startedAt, message, processGroupAlive, processIds }));
`;
const result = spawnSync(

401
test/scripts/mobile-release-process.test-support.ts Normal file
View file

@ -0,0 +1,401 @@
import { ChildProcess } from "node:child_process";
import { channel } from "node:diagnostics_channel";
import fs from "node:fs";
import path from "node:path";
import { clearTimeout as nativeClearTimeout, setTimeout as nativeSetTimeout } from "node:timers";
import { setTimeout as nativeDelay } from "node:timers/promises";
import { expect, it, vi, type TestContext } from "vitest";
import { runBounded } from "../../.github/actions/ios-signing-keychain/keychain.mjs";
import { racePromiseWithAbortSignal } from "../../src/infra/abort-signal.js";
import { hasErrnoCode } from "../../src/infra/errno.js";
import { resolveTestNodeExecPath } from "../../src/test-utils/node-process.js";
import { createFixtureLifetime } from "../helpers/fixture-lifetime.js";
type Kill = typeof process.kill;
type Signal = Parameters<Kill>[1];
type SignalFault = (signal: Signal, forward: () => ReturnType<Kill>) => ReturnType<Kill>;
type Outcome = PromiseSettledResult<Awaited<ReturnType<typeof runBounded>>>;
type NativeBoundedFixture = {
// Verifiers await only this cancellation-aware result; no external async work.
waitForOutcome: () => Promise<Outcome>;
exited: () => boolean;
interceptSignals: (fault: SignalFault) => void;
release: () => void;
assertGroupAbsent: () => void;
};
const testNodeExecPath = resolveTestNodeExecPath();
const JOIN_MS = 2_000;
const nativeNow = Date.now;
async function withinCleanupBudget<T>(promise: Promise<T>, deadline: number, label: string) {
let timer: ReturnType<typeof nativeSetTimeout> | undefined;
try {
return await Promise.race([
promise,
new Promise<never>((_, reject) => {
timer = nativeSetTimeout(
() => reject(new Error(label)),
Math.max(1, deadline - nativeNow()),
);
}),
]);
} finally {
nativeClearTimeout(timer);
}
}
async function withNativeBoundedFixture(
context: Pick<TestContext, "signal" | "onTestFinished">,
options: { source: string; bufferOutputUntilExit?: boolean },
verify: (fixture: NativeBoundedFixture) => Promise<void>,
): Promise<void> {
const lifetime = createFixtureLifetime();
const finished = new AbortController();
const signal = AbortSignal.any([context.signal, finished.signal]);
const originalKill = process.kill.bind(process);
let child: ChildProcess | undefined;
let closed: Promise<void> | undefined;
let childClosed = false;
let childExited = false;
let groupRetired = false;
let outcome: Promise<Outcome> | undefined;
let verification: Promise<void> | undefined;
let root: string | undefined;
let fault: SignalFault | undefined;
let restoreSignalObserver: (() => void) | undefined;
let pendingStop: Promise<void> | undefined;
const inspectGroup = () => {
if (groupRetired || !child?.pid) {
return;
}
try {
originalKill(-child.pid, 0);
} catch (error) {
if (hasErrnoCode(error, "ESRCH")) {
groupRetired = true;
} else if (!hasErrnoCode(error, "EPERM")) {
throw error;
}
}
};
const stop = () =>
(pendingStop ??= lifetime.verifyCleanup(async () => {
finished.abort();
fault = undefined;
const deadline = nativeNow() + JOIN_MS;
try {
if (outcome && !child) {
throw new Error("Native fixture child identity was not captured");
}
// These regression programs contain one process and no descendant launches.
if (child && !childClosed) {
child.kill("SIGKILL");
}
if (closed) {
await withinCleanupBudget(closed, deadline, "Native fixture child did not close");
}
if (child?.pid) {
for (;;) {
inspectGroup();
if (groupRetired) {
break;
}
if (nativeNow() >= deadline) {
throw new Error("Native fixture group exit was not confirmed");
}
await nativeDelay(Math.min(10, Math.max(1, deadline - nativeNow())));
}
}
if (outcome) {
await withinCleanupBudget(outcome, deadline, "Bounded command did not settle");
}
if (verification) {
await withinCleanupBudget(
verification.then(
() => {},
() => {},
),
deadline,
"Native fixture verifier did not settle",
);
}
} finally {
restoreSignalObserver?.();
}
}));
const onAbort = () => {
void stop().catch(() => {});
};
context.signal.addEventListener("abort", onAbort, { once: true });
context.onTestFinished(async () => {
context.signal.removeEventListener("abort", onAbort);
try {
await stop();
} catch (cause) {
// A pending tracked verifier must not turn failed bounded teardown into an
// unbounded lifetime drain. Keep the root and containing resource claim.
throw new Error(`Native fixture cleanup unverified; retained ${root ?? "resource claim"}`, {
cause,
});
}
await lifetime.cleanup();
});
await lifetime.run(async () => {
signal.throwIfAborted();
root = lifetime.createTempDir("ios-keychain-signal-fixture-");
const script = path.join(root, "fixture.cjs");
const gate = path.join(root, "exit");
fs.writeFileSync(script, options.source);
const children = channel("child_process");
const observed: ChildProcess[] = [];
const capture = (message: unknown) => {
if (
message &&
typeof message === "object" &&
"process" in message &&
message.process instanceof ChildProcess
) {
observed.push(message.process);
}
};
children.subscribe(capture);
try {
outcome = runBounded(testNodeExecPath, [script], {
env: { PATH: process.env.PATH, EXIT_GATE: gate, NODE_OPTIONS: "" },
maxOutputBytes: 4_096,
terminateGraceMs: 0,
}).then(
(value) => ({ status: "fulfilled" as const, value }),
(reason: unknown) => ({ status: "rejected" as const, reason }),
);
} finally {
children.unsubscribe(capture);
}
child = observed.find(
(candidate) =>
candidate.spawnargs[0] === testNodeExecPath && candidate.spawnargs[1] === script,
);
try {
if (!child) {
throw new Error("Expected the exact native fixture child");
}
const owned = child;
const commandOutcome = outcome;
closed = new Promise<void>((resolve) => {
owned.once("close", () => {
childClosed = true;
resolve();
});
});
if (options.bufferOutputUntilExit) {
owned.stdout?.pause();
}
owned.once("exit", () => {
childExited = true;
if (options.bufferOutputUntilExit) {
owned.stdout?.resume();
}
});
const pid = owned.pid;
if (typeof pid !== "number" || !Number.isSafeInteger(pid) || pid <= 1) {
throw new Error("Native fixture did not acquire a process group");
}
const signalObserver = vi.spyOn(process, "kill").mockImplementation((target, sig) => {
if (target !== -pid) {
return originalKill(target, sig);
}
const forward = () => {
if (sig !== 0 && childExited && !groupRetired) {
// These fixtures never fork: a group surviving the sole child's exit
// is not fresh authority to signal that numeric PGID again.
inspectGroup();
if (!groupRetired) {
throw new Error("Native fixture group identity unavailable after child exit");
}
}
if (groupRetired) {
throw Object.assign(new Error("retired fixture group"), { code: "ESRCH" });
}
try {
return originalKill(target, sig);
} catch (error) {
if (hasErrnoCode(error, "ESRCH")) {
groupRetired = true;
}
throw error;
}
};
return fault ? fault(sig, forward) : forward();
});
restoreSignalObserver = () => signalObserver.mockRestore();
const fixture: NativeBoundedFixture = {
waitForOutcome: () => racePromiseWithAbortSignal(commandOutcome, signal),
exited: () => childExited,
interceptSignals: (next) => {
signal.throwIfAborted();
fault = next;
},
release: () => {
signal.throwIfAborted();
fs.writeFileSync(gate, "exit\n");
},
assertGroupAbsent: () => {
try {
originalKill(-pid, 0);
} catch (error) {
if (hasErrnoCode(error, "ESRCH")) {
groupRetired = true;
return;
}
throw error;
}
throw new Error("Native fixture group still exists");
},
};
verification = lifetime.track(
Promise.resolve().then(() => {
signal.throwIfAborted();
return verify(fixture);
}),
);
await verification;
} finally {
await stop();
}
});
}
const EXIT_WITH_OUTPUT = 'require("node:fs").writeSync(1, "x".repeat(8192));';
const WAIT_FOR_KILL = [
'process.on("SIGTERM", () => {});',
"setInterval(() => {}, 1000);",
'require("node:fs").writeSync(1, "x".repeat(8192));',
].join("\n");
const WAIT_FOR_RELEASE = [
'const fs = require("node:fs");',
'process.on("SIGTERM", () => {});',
"const timer = setInterval(() => { if (fs.existsSync(process.env.EXIT_GATE)) clearInterval(timer); }, 1);",
'fs.writeSync(1, "x".repeat(8192));',
].join("\n");
function signalError(code: string) {
return Object.assign(new Error(`injected kill ${code}`), { code });
}
// Four independent owner/error contracts; none adds a production injection seam.
export function registerBoundedSignalTests() {
if (process.platform === "win32") {
return;
}
for (const persistent of [false, true]) {
it(`requires confirmed process-group disappearance after ${persistent ? "persistent" : "transient"} EPERM`, async (context) => {
await withNativeBoundedFixture(context, { source: WAIT_FOR_KILL }, async (fixture) => {
let probes = 0;
let nonzeroAfterFault = 0;
const permission = signalError("EPERM");
fixture.interceptSignals((signal, forward) => {
if (signal !== 0) {
if (probes) {
nonzeroAfterFault += 1;
}
return forward();
}
try {
return forward();
} catch (error) {
if (!hasErrnoCode(error, "ESRCH")) {
throw error;
}
probes += 1;
if (persistent || probes <= 2) {
throw permission;
}
throw error;
}
});
const outcome = await fixture.waitForOutcome();
expect(outcome.status).toBe("rejected");
if (outcome.status !== "rejected") {
throw new Error("Expected bounded command rejection");
}
if (persistent) {
expect(outcome.reason).toMatchObject({
message: expect.stringContaining("owned process group did not terminate"),
cause: permission,
});
} else {
expect(outcome.reason).toMatchObject({
message: expect.stringContaining("exceeded the 4096-byte output limit"),
});
}
expect(probes).toBeGreaterThanOrEqual(3);
expect(nonzeroAfterFault).toBe(0);
fixture.assertGroupAbsent();
});
});
}
it("preserves the output-limit failure when the group exits before signaling", async (context) => {
await withNativeBoundedFixture(
context,
{ source: EXIT_WITH_OUTPUT, bufferOutputUntilExit: true },
async (fixture) => {
let permissionFaults = 0;
let exitedBeforeSignal = false;
fixture.interceptSignals((signal, forward) => {
try {
return forward();
} catch (error) {
if (signal === "SIGTERM" && hasErrnoCode(error, "ESRCH")) {
exitedBeforeSignal = fixture.exited();
permissionFaults += 1;
throw signalError("EPERM");
}
throw error;
}
});
const outcome = await fixture.waitForOutcome();
expect(outcome).toMatchObject({
status: "rejected",
reason: { message: expect.stringContaining("exceeded the 4096-byte output limit") },
});
expect(exitedBeforeSignal).toBe(true);
expect(permissionFaults).toBe(1);
fixture.assertGroupAbsent();
},
);
});
it("preserves a hard signaling error after provisional EPERM", async (context) => {
await withNativeBoundedFixture(context, { source: WAIT_FOR_RELEASE }, async (fixture) => {
const permission = signalError("EPERM");
const ioFailure = signalError("EIO");
const signals: Array<number | string | undefined> = [];
fixture.interceptSignals((signal, forward) => {
if (signal === "SIGTERM") {
signals.push(signal);
throw permission;
}
if (signal === "SIGKILL") {
signals.push(signal);
fixture.release();
throw ioFailure;
}
return forward();
});
const outcome = await fixture.waitForOutcome();
expect(outcome.status).toBe("rejected");
if (outcome.status !== "rejected") {
throw new Error("Expected bounded command rejection");
}
expect(outcome.reason).toBe(ioFailure);
expect(signals).toEqual(["SIGTERM", "SIGKILL"]);
fixture.assertGroupAbsent();
});
});
}