mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): stop macOS shell groups before killing them
Reduce the race where killing a foreground child wakes its shell before the shell is signalled. Keep Linux and Windows termination unchanged and preserve kill fallbacks when stopping fails. closes #11252
This commit is contained in:
parent
74f4686402
commit
27cc6bfc0c
2 changed files with 88 additions and 0 deletions
|
|
@ -236,6 +236,19 @@ export function killProcessTree(pid: number): void {
|
|||
// Ignore errors if taskkill fails.
|
||||
}
|
||||
} else {
|
||||
if (process.platform === "darwin") {
|
||||
// macOS signals group members individually after releasing the group lock. A killed
|
||||
// child can wake its waiting shell before that shell is signalled, letting `a; b`
|
||||
// start b during cancellation (#11252). Stop the group first to reduce this race;
|
||||
// this is best-effort, not an atomic freeze. SIGKILL also kills stopped processes.
|
||||
// Linux holds tasklist_lock across group signalling; child exit notification needs
|
||||
// its write lock, so the shell is signalled before observing the killed child's exit.
|
||||
try {
|
||||
process.kill(-pid, "SIGSTOP");
|
||||
} catch {
|
||||
// A failed stop must not prevent the kill attempt or its single-process fallback.
|
||||
}
|
||||
}
|
||||
// Use SIGKILL on Unix/Linux/Mac
|
||||
try {
|
||||
process.kill(-pid, "SIGKILL");
|
||||
|
|
|
|||
75
packages/coding-agent/test/shell-kill-process-tree.test.ts
Normal file
75
packages/coding-agent/test/shell-kill-process-tree.test.ts
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { killProcessTree } from "../src/utils/shell.ts";
|
||||
|
||||
const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform");
|
||||
|
||||
// Regression coverage for #11252: stop-before-kill is macOS-only and best-effort.
|
||||
describe("killProcessTree", () => {
|
||||
beforeEach(() => {
|
||||
Object.defineProperty(process, "platform", { configurable: true, value: "darwin" });
|
||||
vi.spyOn(process, "kill").mockReturnValue(true);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
if (platformDescriptor) Object.defineProperty(process, "platform", platformDescriptor);
|
||||
});
|
||||
|
||||
it("stops the macOS process group before killing it without resuming it", () => {
|
||||
killProcessTree(1234);
|
||||
|
||||
expect(vi.mocked(process.kill).mock.calls).toEqual([
|
||||
[-1234, "SIGSTOP"],
|
||||
[-1234, "SIGKILL"],
|
||||
]);
|
||||
});
|
||||
|
||||
it("still kills the group when stopping fails", () => {
|
||||
vi.mocked(process.kill).mockImplementationOnce(() => {
|
||||
throw new Error("stop failed");
|
||||
});
|
||||
|
||||
killProcessTree(1234);
|
||||
|
||||
expect(vi.mocked(process.kill).mock.calls).toEqual([
|
||||
[-1234, "SIGSTOP"],
|
||||
[-1234, "SIGKILL"],
|
||||
]);
|
||||
});
|
||||
|
||||
it("falls back to killing the shell when the group kill fails", () => {
|
||||
vi.mocked(process.kill).mockImplementation((pid, signal) => {
|
||||
if (pid < 0 && signal === "SIGKILL") throw new Error("group kill failed");
|
||||
return true;
|
||||
});
|
||||
|
||||
killProcessTree(1234);
|
||||
|
||||
expect(vi.mocked(process.kill).mock.calls).toEqual([
|
||||
[-1234, "SIGSTOP"],
|
||||
[-1234, "SIGKILL"],
|
||||
[1234, "SIGKILL"],
|
||||
]);
|
||||
});
|
||||
|
||||
it("tolerates failure of both group signals and the fallback", () => {
|
||||
vi.mocked(process.kill).mockImplementation(() => {
|
||||
throw new Error("process no longer exists");
|
||||
});
|
||||
|
||||
expect(() => killProcessTree(1234)).not.toThrow();
|
||||
expect(vi.mocked(process.kill).mock.calls).toEqual([
|
||||
[-1234, "SIGSTOP"],
|
||||
[-1234, "SIGKILL"],
|
||||
[1234, "SIGKILL"],
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps Linux group termination unchanged", () => {
|
||||
Object.defineProperty(process, "platform", { configurable: true, value: "linux" });
|
||||
|
||||
killProcessTree(1234);
|
||||
|
||||
expect(vi.mocked(process.kill).mock.calls).toEqual([[-1234, "SIGKILL"]]);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue