fix(exec): stop bash shell supervisor busy-spinning while idle (#5972)

packages/coding-agent/changelog.d/5972-supervisor-idle-cpu.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- The isolated bash shell supervisor no longer busy-spins while its shell is idle. It reaped adopted zombies by sweeping all of `/proc` every 25 ms without waiting for the previous sweep to finish, which kept each supervisor at about 160% CPU and tens of thousands of context switches per second even during `sleep`. Sweeps now run only on `SIGCHLD`, one at a time, so an idle supervisor stays near 0% CPU (#5972).

packages/coding-agent/src/exec/bash-shell-supervisor.ts

@@ -1,6 +1,7 @@
 import { dlopen, FFIType, ptr } from "bun:ffi";
 import * as childProcess from "node:child_process";
 import { createHmac } from "node:crypto";
+import type { EventEmitter } from "node:events";
 import * as fs from "node:fs/promises";
 import * as os from "node:os";
 import * as path from "node:path";
@@ -121,6 +122,63 @@ async function reapLinuxAdoptedZombies(excludedPid: number | undefined): Promise
 	}
 }
 
+/**
+ * Reap adopted zombies only when the kernel reports a child state change.
+ *
+ * As the Linux child subreaper, the supervisor inherits orphaned descendants
+ * and must reap them, but a full `/proc` sweep is expensive. Polling it on a
+ * short interval kept every idle supervisor busy (issue #5972), so sweeps are
+ * driven by SIGCHLD instead. At most one sweep runs at a time, and a SIGCHLD that
+ * arrives mid-sweep schedules exactly one follow-up so a zombie created after
+ * its `/proc` entry was passed is never missed. Returns the unsubscribe hook.
+ */
+export function startLinuxAdoptedZombieReaper(
+	excludedPid: number | undefined,
+	reap: (excludedPid: number | undefined) => Promise<void> = reapLinuxAdoptedZombies,
+	signals: Pick<EventEmitter, "on" | "off"> = process,
+): () => void {
+	if (process.platform !== "linux") return () => undefined;
+	let running = false;
+	let rerun = false;
+	let stopped = false;
+	let sweepPending = false;
+	const sweep = (): void => {
+		if (stopped) return;
+		if (running) {
+			rerun = true;
+			return;
+		}
+		if (sweepPending) return;
+		sweepPending = true;
+		running = true;
+		void reap(excludedPid)
+			.catch(() => undefined)
+			.finally(() => {
+				running = false;
+				sweepPending = false;
+				if (!rerun || stopped) return;
+				rerun = false;
+				// Defer recursive call to avoid stack overflow
+				setImmediate(() => {
+					if (!stopped && !running) sweep();
+				});
+			});
+	};
+	try {
+		signals.on("SIGCHLD", sweep);
+	} catch {
+		// Signal handler installation may fail in some environments
+	}
+	return () => {
+		stopped = true;
+		try {
+			signals.off("SIGCHLD", sweep);
+		} catch {
+			// Signal handler removal may fail in some environments
+		}
+	};
+}
+
 function signalFromExit(code: number | null, signal: NodeJS.Signals | null): NodeJS.Signals | null {
 	if (signal) return signal;
 	if (code === null || code <= 128) return null;
@@ -210,8 +268,7 @@ export async function runBashShellSupervisor(): Promise<void> {
 		stdio: ["pipe", "pipe", "pipe"],
 		windowsHide: true,
 	});
-	const zombieReaper = setInterval(() => void reapLinuxAdoptedZombies(worker.pid), 25);
-	zombieReaper.unref();
+	const stopZombieReaper = startLinuxAdoptedZombieReaper(worker.pid);
 	const pendingRuns: number[] = [];
 	let terminating = false;
 	let protocolToken: string | undefined;
@@ -315,7 +372,7 @@ export async function runBashShellSupervisor(): Promise<void> {
 		);
 	});
 	worker.once("exit", async (code, signal) => {
-		clearInterval(zombieReaper);
+		stopZombieReaper();
 		const resultSignal = signalFromExit(code, signal);
 		if (code === 0 && resultSignal === null) {
 			await reapLinuxSubreaperChildren();

packages/coding-agent/test/bash-shell-supervisor-idle.test.ts

@@ -0,0 +1,148 @@
+import { afterEach, describe, expect, it } from "bun:test";
+import { EventEmitter } from "node:events";
+import * as fs from "node:fs";
+import * as os from "node:os";
+import * as path from "node:path";
+import { startLinuxAdoptedZombieReaper } from "@gajae-code/coding-agent/exec/bash-shell-supervisor";
+import { IsolatedShell } from "@gajae-code/coding-agent/exec/isolated-shell";
+
+const isLinux = process.platform === "linux";
+// Linux reports utime/stime in USER_HZ, which is 100 on every supported target.
+const CLOCK_TICKS_PER_SECOND = 100;
+
+type ProcSample = { cpuSeconds: number; contextSwitches: number; at: number };
+
+function sampleProc(pid: number): ProcSample {
+	const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8");
+	const fields = stat
+		.slice(stat.lastIndexOf(")") + 2)
+		.trim()
+		.split(" ");
+	// Fields after `comm` start at `state` (field 3); utime/stime are fields 14/15.
+	const cpuTicks = Number(fields[11]) + Number(fields[12]);
+	const status = fs.readFileSync(`/proc/${pid}/status`, "utf8");
+	let contextSwitches = 0;
+	for (const match of status.matchAll(/ctxt_switches:\s+(\d+)/g)) contextSwitches += Number(match[1]);
+	return { cpuSeconds: cpuTicks / CLOCK_TICKS_PER_SECOND, contextSwitches, at: performance.now() };
+}
+
+function procState(pid: number): string | undefined {
+	try {
+		const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8");
+		return stat.slice(stat.lastIndexOf(")") + 2).split(" ")[0];
+	} catch {
+		return undefined;
+	}
+}
+
+describe("bash shell supervisor idle behavior (issue #5972)", () => {
+	const shells: IsolatedShell[] = [];
+	const dirs: string[] = [];
+
+	afterEach(async () => {
+		await Promise.all(shells.splice(0).map(shell => shell.close().catch(() => undefined)));
+		for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true });
+	});
+
+	it.skipIf(!isLinux)(
+		"stays near 0% CPU while its child shell is idle in sleep",
+		async () => {
+			const shell = new IsolatedShell();
+			shells.push(shell);
+			await shell.ready();
+			const supervisorPid = shell.supervisorPid();
+			expect(supervisorPid).toBeNumber();
+
+			const running = shell.run({ command: "sleep 3", cwd: os.tmpdir() });
+			// Let the run dispatch and startup work settle before sampling.
+			await Bun.sleep(500);
+			const before = sampleProc(supervisorPid!);
+			await Bun.sleep(2_000);
+			const after = sampleProc(supervisorPid!);
+			const wallSeconds = (after.at - before.at) / 1000;
+			const cpuPercent = ((after.cpuSeconds - before.cpuSeconds) / wallSeconds) * 100;
+			const switchesPerSecond = (after.contextSwitches - before.contextSwitches) / wallSeconds;
+
+			// The 25 ms /proc polling loop measured ~160% CPU and ~30-70k context
+			// switches/s here. An event-driven supervisor idles well under 1%.
+			expect(cpuPercent).toBeLessThan(10);
+			expect(switchesPerSecond).toBeLessThan(2_000);
+			await expect(running).resolves.toMatchObject({ exitCode: 0, cancelled: false });
+		},
+		15_000,
+	);
+
+	it.skipIf(!isLinux)(
+		"still reaps a descendant reparented to the supervisor once it exits",
+		async () => {
+			const dir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-supervisor-reap-"));
+			dirs.push(dir);
+			const pidFile = path.join(dir, "grandchild.pid");
+			const shell = new IsolatedShell();
+			shells.push(shell);
+			await shell.ready();
+			const supervisorPid = shell.supervisorPid();
+
+			// The intermediate `sh` exits immediately, so the backgrounded sleep is
+			// orphaned and adopted by the supervisor (the Linux child subreaper).
+			await shell.run({ command: `sh -c 'sleep 0.3 & echo $! > "${pidFile}"'`, cwd: dir });
+			const grandchildPid = Number.parseInt(fs.readFileSync(pidFile, "utf8"), 10);
+			expect(grandchildPid).toBeGreaterThan(0);
+
+			const deadline = Date.now() + 5_000;
+			while (procState(grandchildPid) !== undefined && Date.now() < deadline) await Bun.sleep(25);
+			// Neither alive nor left behind as an unreaped zombie of the supervisor.
+			expect(procState(grandchildPid)).toBeUndefined();
+			expect(procState(supervisorPid!)).not.toBe("Z");
+		},
+		15_000,
+	);
+
+	it.skipIf(!isLinux)("runs one sweep per SIGCHLD burst and never overlaps sweeps", async () => {
+		let sweeps = 0;
+		let active = 0;
+		let maxActive = 0;
+		let release = Promise.withResolvers<void>();
+		// A private emitter isolates the scheduler from real SIGCHLDs that this
+		// test process receives when other tests' children exit.
+		const signals = new EventEmitter();
+		const stop = startLinuxAdoptedZombieReaper(
+			undefined,
+			async () => {
+				sweeps++;
+				active++;
+				maxActive = Math.max(maxActive, active);
+				await release.promise;
+				active--;
+			},
+			signals,
+		);
+		try {
+			// No child state change: no sweep, regardless of elapsed time.
+			await Bun.sleep(100);
+			expect(sweeps).toBe(0);
+
+			signals.emit("SIGCHLD");
+			expect(sweeps).toBe(1);
+			// A burst during an in-flight sweep coalesces into exactly one follow-up.
+			for (let i = 0; i < 10; i++) signals.emit("SIGCHLD");
+			expect(sweeps).toBe(1);
+			const first = release;
+			release = Promise.withResolvers<void>();
+			first.resolve();
+			await Bun.sleep(10);
+			expect(sweeps).toBe(2);
+			release.resolve();
+			await Bun.sleep(10);
+			expect(sweeps).toBe(2);
+			expect(maxActive).toBe(1);
+
+			stop();
+			expect(signals.listenerCount("SIGCHLD")).toBe(0);
+			signals.emit("SIGCHLD");
+			expect(sweeps).toBe(2);
+		} finally {
+			stop();
+		}
+	});
+});