mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 20:35:39 -06:00
fix(android): stop wrapper subprocess trees on cancellation (#124686)
* fix(android): reap Gradle wrapper subprocesses * fix(android): preserve wrapper spawn errors
This commit is contained in:
committed by
GitHub
parent
eb63cb1481
commit
337933509d
@@ -1,9 +1,9 @@
|
||||
#!/usr/bin/env node
|
||||
import { spawn } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { runManagedCommand } from "./lib/managed-child-process.mts";
|
||||
import { resolveRepoRoot } from "./lib/repo-root.mjs";
|
||||
|
||||
const repoRoot = resolveRepoRoot(import.meta.url);
|
||||
@@ -74,32 +74,24 @@ export function resolveAndroidSdkEnv(options: SdkOptions = {}) {
|
||||
return { ...env, ANDROID_HOME: defaultSdkDir };
|
||||
}
|
||||
|
||||
export function run(
|
||||
export async function run(
|
||||
command: string,
|
||||
args: readonly string[],
|
||||
cwd: string,
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
) {
|
||||
return new Promise<number>((resolve) => {
|
||||
const child = spawn(command, args, {
|
||||
try {
|
||||
return await runManagedCommand({
|
||||
args: [...args],
|
||||
bin: command,
|
||||
cwd,
|
||||
env,
|
||||
stdio: "inherit",
|
||||
shell: false,
|
||||
});
|
||||
child.on("close", (status, signal) => {
|
||||
if (typeof status === "number") {
|
||||
resolve(status);
|
||||
} else if (signal) {
|
||||
resolve(128);
|
||||
} else {
|
||||
resolve(1);
|
||||
}
|
||||
});
|
||||
child.on("error", (error) => {
|
||||
console.error(error instanceof Error ? error.message : String(error));
|
||||
resolve(1);
|
||||
});
|
||||
});
|
||||
} catch (error) {
|
||||
console.error(error instanceof Error ? error.message : String(error));
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
|
||||
export async function main(argv: string[] = process.argv.slice(2)) {
|
||||
|
||||
@@ -1,11 +1,21 @@
|
||||
import { spawn } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { setTimeout as delay } from "node:timers/promises";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
linuxArmAndroidGradleSkipMessage,
|
||||
resolveAndroidSdkEnv,
|
||||
run,
|
||||
shouldSkipLinuxArmAndroidGradle,
|
||||
splitAndroidGradleArgs,
|
||||
} from "../../scripts/run-android-gradle.mts";
|
||||
import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";
|
||||
|
||||
const posixIt = process.platform === "win32" ? it.skip : it;
|
||||
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
|
||||
describe("run-android-gradle", () => {
|
||||
it("splits Gradle args from an optional post command", () => {
|
||||
@@ -90,4 +100,124 @@ describe("run-android-gradle", () => {
|
||||
expect(result).toBe(env);
|
||||
});
|
||||
});
|
||||
|
||||
posixIt("terminates the active command tree when the wrapper is terminated", async () => {
|
||||
const dir = tempDirs.make("openclaw-android-gradle-process-");
|
||||
const processTreePath = path.join(dir, "process-tree.json");
|
||||
const moduleUrl = pathToFileURL(path.resolve("scripts/run-android-gradle.mts")).href;
|
||||
const childSource = `
|
||||
const { spawn } = require("node:child_process");
|
||||
const fs = require("node:fs");
|
||||
const descendant = spawn(process.execPath, [
|
||||
"-e",
|
||||
"process.on('SIGTERM', () => {}); setInterval(() => {}, 1_000);",
|
||||
], { stdio: "ignore" });
|
||||
fs.writeFileSync(
|
||||
process.argv[1],
|
||||
JSON.stringify({ childPid: process.pid, descendantPid: descendant.pid }),
|
||||
);
|
||||
setInterval(() => {}, 1_000);
|
||||
`;
|
||||
const runnerSource = `
|
||||
import { run } from ${JSON.stringify(moduleUrl)};
|
||||
process.exitCode = await run(
|
||||
process.execPath,
|
||||
["-e", ${JSON.stringify(childSource)}, ${JSON.stringify(processTreePath)}],
|
||||
process.cwd(),
|
||||
);
|
||||
`;
|
||||
const runner = spawn(
|
||||
process.execPath,
|
||||
["--import", "tsx", "--input-type=module", "-e", runnerSource],
|
||||
{ stdio: "ignore" },
|
||||
);
|
||||
const runnerPid = expectPid(runner.pid);
|
||||
let childPid = 0;
|
||||
let descendantPid = 0;
|
||||
|
||||
try {
|
||||
await waitFor(() => fs.existsSync(processTreePath));
|
||||
const processTree = JSON.parse(fs.readFileSync(processTreePath, "utf8")) as {
|
||||
childPid: number;
|
||||
descendantPid: number;
|
||||
};
|
||||
childPid = processTree.childPid;
|
||||
descendantPid = processTree.descendantPid;
|
||||
expect(Number.isInteger(childPid)).toBe(true);
|
||||
expect(Number.isInteger(descendantPid)).toBe(true);
|
||||
expect(isProcessAlive(childPid)).toBe(true);
|
||||
expect(isProcessAlive(descendantPid)).toBe(true);
|
||||
|
||||
process.kill(runnerPid, "SIGTERM");
|
||||
const result = await waitForClose(runner);
|
||||
await waitFor(() => !isProcessAlive(childPid), 1_500);
|
||||
await waitFor(() => !isProcessAlive(descendantPid), 1_500);
|
||||
|
||||
expect(isProcessAlive(childPid)).toBe(false);
|
||||
expect(isProcessAlive(descendantPid)).toBe(false);
|
||||
expect(result).toEqual({ code: 143, signal: null });
|
||||
} finally {
|
||||
if (isProcessAlive(runnerPid)) {
|
||||
process.kill(runnerPid, "SIGKILL");
|
||||
}
|
||||
if (childPid && isProcessAlive(childPid)) {
|
||||
process.kill(childPid, "SIGKILL");
|
||||
}
|
||||
if (descendantPid && isProcessAlive(descendantPid)) {
|
||||
process.kill(descendantPid, "SIGKILL");
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("reports spawn errors and returns a failure status", async () => {
|
||||
const error = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
const missingCommand = path.join(os.tmpdir(), `openclaw-missing-command-${process.pid}`);
|
||||
try {
|
||||
await expect(run(missingCommand, [], process.cwd(), {})).resolves.toBe(1);
|
||||
expect(error).toHaveBeenCalledOnce();
|
||||
expect(String(error.mock.calls[0]?.[0])).toContain("ENOENT");
|
||||
} finally {
|
||||
error.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
function expectPid(pid: number | undefined): number {
|
||||
if (pid === undefined) {
|
||||
throw new Error("expected child process pid");
|
||||
}
|
||||
return pid;
|
||||
}
|
||||
|
||||
async function waitFor(condition: () => boolean, timeoutMs = 3_000): Promise<void> {
|
||||
const startedAt = Date.now();
|
||||
while (!condition()) {
|
||||
if (Date.now() - startedAt > timeoutMs) {
|
||||
throw new Error("timed out waiting for condition");
|
||||
}
|
||||
await delay(5);
|
||||
}
|
||||
}
|
||||
|
||||
async function waitForClose(child: ReturnType<typeof spawn>) {
|
||||
return await new Promise<{ code: number | null; signal: NodeJS.Signals | null }>((resolve) => {
|
||||
child.once("close", (code, signal) => resolve({ code, signal }));
|
||||
});
|
||||
}
|
||||
|
||||
function isProcessAlive(pid: number): boolean {
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
if (process.platform !== "linux") {
|
||||
return true;
|
||||
}
|
||||
try {
|
||||
const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8");
|
||||
return stat.charAt(stat.lastIndexOf(")") + 2) !== "Z";
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user