Best effort alternative to tree-kill that swallows errors

node-src/lib/shell/shell.test.ts

@@ -0,0 +1,26 @@
+import { describe, expect, it } from 'vitest';
+
+import { runCommand } from './shell';
+
+describe('runCommand', () => {
+  it('returns stdout on success', async () => {
+    const result = await runCommand('echo hello');
+    expect(result.stdout).toBe('hello');
+  });
+
+  it('kill() terminates the process', async () => {
+    const proc = runCommand('sleep 60');
+    await new Promise((r) => setTimeout(r, 200));
+
+    expect(proc.pid).toBeDefined();
+    proc.kill();
+
+    const error: any = await proc.catch((error) => error);
+    expect(error.signal).toBe('SIGKILL');
+    expect(error.isTerminated).toBe(true);
+  });
+
+  it('times out and kills the process', async () => {
+    await expect(runCommand('sleep 60', { timeout: 200 })).rejects.toThrow(/timed out/);
+  });
+});

node-src/lib/shell/shell.ts

@@ -1,5 +1,6 @@
 import { execa, Options, parseCommandString, type ResultPromise } from 'execa';
-import treeKill from 'tree-kill';
+
+import { treeKill } from './treeKill';
 
 /**
  * Run a command in the shell. This is a wrapper around `execa` so .kill() and timeouts kill the
@@ -19,14 +20,25 @@ export function runCommand(command: string, options: Options = {}): ResultPromis
   const [cmd, ...args] = parseCommandString(command);
   const subprocess = execa(cmd, args, optionsWithoutTimeout);
 
-  // Override subprocess.kill() because we want to kill the entire process tree on timeout instead
-  // of just the child process created by `execa`
   subprocess.kill = () => {
     if (!subprocess.pid) {
       return false;
     }
+    const pid = subprocess.pid;
 
-    treeKill(subprocess.pid);
+    // Unfortunately, we can't change the signature of .kill() to be async, so we have to fire and forget the tree kill.
+    // This means that if the tree kill fails completely, we won't know about it, but it's better than leaving orphaned processes.
+    treeKill(pid)
+      .catch(() => {
+        /* noop - prevent unhandled promise rejection  */
+      })
+      .finally(() => {
+        try {
+          process.kill(pid, 'SIGKILL');
+        } catch {
+          /* noop - process may already be dead, but we tried our best to kill it */
+        }
+      });
     return true;
   };
 

node-src/lib/shell/treeKill.test.ts

@@ -0,0 +1,77 @@
+import { type ChildProcess, spawn } from 'child_process';
+import { afterEach, describe, expect, it } from 'vitest';
+
+import { treeKill } from './treeKill';
+
+function pidIsAlive(pid: number) {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * Spawns a Node parent process that itself spawns a `sleep 60` child.
+ * Returns handles to both so we can assert on their lifecycle independently.
+ *
+ * @returns
+ */
+function spawnTree(): Promise<{ parent: ChildProcess; childPid: number }> {
+  return new Promise((resolve, reject) => {
+    const parent = spawn('node', [
+      '-e',
+      `
+      const { spawn } = require('child_process');
+      const child = spawn('sleep', ['60']);
+      process.stdout.write('child:' + child.pid + '\\n');
+      setInterval(() => {}, 60000);
+      `,
+    ]);
+
+    parent.stdout?.on('data', (data) => {
+      const match = data.toString().match(/child:(\d+)/);
+      if (match) {
+        resolve({ parent, childPid: Number(match[1]) });
+      }
+    });
+
+    parent.on('error', reject);
+  });
+}
+
+const processesToCleanup: ChildProcess[] = [];
+
+afterEach(() => {
+  for (const proc of processesToCleanup) {
+    try {
+      proc.kill('SIGKILL');
+    } catch {
+      // already dead
+    }
+  }
+  processesToCleanup.length = 0;
+});
+
+describe('treeKill', () => {
+  it('kills child processes but not the root pid', async () => {
+    const { parent, childPid } = await spawnTree();
+    processesToCleanup.push(parent);
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    const parentPid = parent.pid!;
+
+    expect(pidIsAlive(parentPid)).toBe(true);
+    expect(pidIsAlive(childPid)).toBe(true);
+
+    await treeKill(parentPid);
+    await new Promise((r) => setTimeout(r, 100));
+
+    expect(pidIsAlive(childPid)).toBe(false);
+    expect(pidIsAlive(parentPid)).toBe(true);
+  });
+
+  it('does not throw for a non-existent pid', async () => {
+    await expect(treeKill(2_147_483_647)).resolves.toBeUndefined();
+  });
+});

node-src/lib/shell/treeKill.ts

@@ -0,0 +1,46 @@
+import { exec } from 'child_process';
+import { readFileSync } from 'fs';
+import { promisify } from 'util';
+
+const pExec = promisify(exec);
+
+/**
+ * Best effort process tree killing from a pid.
+ *
+ * @param pid Root process ID
+ * @param signal Signal to send to the processes, default is 'SIGKILL'
+ */
+export async function treeKill(pid: number, signal: NodeJS.Signals = 'SIGKILL') {
+  if (process.platform === 'win32') {
+    await pExec(`taskkill /pid ${pid} /T /F`);
+  } else {
+    let pids = [];
+    pids = process.platform === 'darwin' ? await darwinGetChildPids(pid) : linuxGetChildPids(pid);
+    for (const child_pid of pids) {
+      process.kill(child_pid, signal);
+    }
+  }
+}
+
+function linuxGetChildPids(pid: number) {
+  try {
+    const children = readFileSync(`/proc/${pid}/task/${pid}/children`, 'utf8');
+    const pids = children.split(' ').filter(Boolean).map(Number);
+    return [pids, ...pids.map((child_pid) => linuxGetChildPids(child_pid))].flat();
+  } catch {
+    return [];
+  }
+}
+
+async function darwinGetChildPids(pid: number) {
+  try {
+    const { stdout } = await pExec('pgrep -P ' + pid);
+    const pids = stdout.split('\n').filter(Boolean).map(Number);
+    for (const child_pid of pids) {
+      pids.push(...(await darwinGetChildPids(child_pid)));
+    }
+    return pids;
+  } catch {
+    return [];
+  }
+}

package.json

@@ -200,7 +200,6 @@
     "string-argv": "^0.3.1",
     "strip-ansi": "^7.1.0",
     "tmp-promise": "3.0.2",
-    "tree-kill": "^1.2.2",
     "ts-dedent": "^1.0.0",
     "ts-loader": "^9.2.5",
     "tsup": "^7.2.0",

yarn.lock

@@ -8120,7 +8120,6 @@ __metadata:
     string-argv: "npm:^0.3.1"
     strip-ansi: "npm:^7.1.0"
     tmp-promise: "npm:3.0.2"
-    tree-kill: "npm:^1.2.2"
     ts-dedent: "npm:^1.0.0"
     ts-loader: "npm:^9.2.5"
     tsup: "npm:^7.2.0"