diff --git a/README.md b/README.md index cc284b6..daa817c 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,8 @@ The options object can have the following properties: - `signal` - an `AbortSignal` to allow aborting of the execution - `timeout` - time in milliseconds at which the process will be forcibly killed - `persist` - if `true`, the process will continue after the host exits +- `killDescendants` - if `true`, terminating the process will also terminate + its descendants (defaults to `false`) - `stdin` - `string` or another `Result` that will be used as the input to the process - `nodeOptions` - any valid options to node's underlying `spawn` function - `throwOnError` - if true, non-zero exit codes will throw an error @@ -111,6 +113,37 @@ proc.kill(); proc.kill('SIGHUP'); ``` +### Killing descendant processes + +By default, terminating a process only terminates the direct child. Any +processes spawned by that child can continue running. + +Pass `killDescendants: true` to terminate the process tree when `kill()` is +called, a timeout expires, or an `AbortSignal` aborts: + +```ts +const proc = x('node', ['./server.mjs'], { + killDescendants: true +}); + +proc.kill(); +await proc; +``` + +On Unix, tinyexec starts the subprocess in its own process group and signals +that group. On Windows, it uses `taskkill /T /F`. This is best-effort: +descendants that create their own process group or session are not terminated, +and Windows falls back to terminating only the direct child if `taskkill` +cannot be used. + +On Unix, this option needs the subprocess to lead its own process group, so it +implies `detached: true`. This has the same effects as `persist`: the +subprocess keeps running after the host exits, and terminal signals such as +`CTRL-C` are not forwarded to it automatically. Any `nodeOptions.detached` +value you set is overridden. + +Like `persist`, this option is not supported by the synchronous API. + ### Node modules/binaries By default, node's available binaries from `node_modules` will be accessible @@ -197,6 +230,7 @@ Since the synchronous API blocks the event loop, there are some features that ar - `signal` - `persist` +- `killDescendants` - `kill()` method - `stdin` piping - `pipe()` method diff --git a/THIRD_PARTY_LICENSES.txt b/THIRD_PARTY_LICENSES.txt index be276a3..2f25519 100644 --- a/THIRD_PARTY_LICENSES.txt +++ b/THIRD_PARTY_LICENSES.txt @@ -21,3 +21,27 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +# execa + +MIT License + +Copyright (c) Sindre Sorhus (https://sindresorhus.com) + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/src/kill-descendants.ts b/src/kill-descendants.ts new file mode 100644 index 0000000..a0bf59e --- /dev/null +++ b/src/kill-descendants.ts @@ -0,0 +1,102 @@ +import { + type ChildProcess, + execFile, + type SpawnOptions +} from 'node:child_process'; +import { + join as joinWindowsPath, + parse as parseWindowsPath +} from 'node:path/win32'; + +type KillSignal = Parameters[0]; +type KillFunction = (signal?: KillSignal) => boolean; + +const isWindows = process.platform === 'win32'; +const isWindowsDriveRootRegExp = /^[a-z]:[\\/]/i; + +/** + * Makes the subprocess lead its own process group, so a negative PID can + * later signal the whole group. Windows has no process groups, so the + * options come back untouched there. + */ +export function detachProcessGroup(options: SpawnOptions): SpawnOptions { + return isWindows ? options : {...options, detached: true}; +} + +/** + * Creates a replacement for `subprocess.kill` which takes the descendants + * down too. It's best-effort: descendants which start their own group or + * session survive, and Windows falls back to killing the direct child. + */ +export function createKillFunction( + subprocess: ChildProcess, + env: NodeJS.ProcessEnv = process.env +): KillFunction { + // Capture the original method before the caller replaces it + const kill = subprocess.kill.bind(subprocess); + + return (signal): boolean => { + // Signal 0 is a liveness probe, not a termination. + if (signal === 0) { + return kill(signal); + } + + if (subprocess.pid === undefined) { + return false; + } + + return isWindows + ? killWindowsTree(subprocess.pid, signal, kill, env) + : killProcessGroup(subprocess.pid, signal, kill); + }; +} + +function killWindowsTree( + pid: number, + signal: KillSignal, + kill: KillFunction, + env: NodeJS.ProcessEnv +): boolean { + const taskkillFile = resolveTaskkillPath(env); + + if (taskkillFile === undefined) { + return kill(signal); + } + + // taskkill must run before the direct child exits, or its descendants + // might be orphaned before Windows can enumerate the process tree. + execFile(taskkillFile, ['/pid', String(pid), '/T', '/F'], (error) => { + if (error) { + kill(signal); + } + }); + + return true; +} + +function killProcessGroup( + pid: number, + signal: KillSignal, + kill: KillFunction +): boolean { + try { + // A negative PID signals the whole process group on Unix. + return process.kill(-pid, signal); + } catch { + // group's gone, or we're not allowed to signal it + return kill(signal); + } +} + +function resolveTaskkillPath(env: NodeJS.ProcessEnv): string | undefined { + // Resolve the system binary directly instead of relying on PATH. + const windowsDirectory = [env.SystemRoot, env.windir].find( + (directory): directory is string => + directory !== undefined && + isWindowsDriveRootRegExp.test(parseWindowsPath(directory).root) + ); + + return windowsDirectory === undefined + ? undefined + : joinWindowsPath(windowsDirectory, 'System32', 'taskkill.exe'); +} diff --git a/src/main.ts b/src/main.ts index 424d1e7..1574a9d 100644 --- a/src/main.ts +++ b/src/main.ts @@ -12,6 +12,7 @@ import {combineStreams} from './stream.js'; import readline from 'node:readline'; import {normalizeSpawnCommand} from './normalize.js'; import {NonZeroExitError} from './non-zero-exit-error.js'; +import {createKillFunction, detachProcessGroup} from './kill-descendants.js'; export {NonZeroExitError, normalizeSpawnCommand}; @@ -64,6 +65,7 @@ export interface Options extends CommonOptions { nodeOptions: SpawnOptions; persist: boolean; stdin: Result | ExecProcess | string; + killDescendants: boolean; } export interface SyncOptions extends CommonOptions { @@ -80,7 +82,8 @@ export interface TinyExec { const defaultOptions = { timeout: undefined, - persist: false + persist: false, + killDescendants: false } satisfies Partial; const defaultSyncOptions = { @@ -128,6 +131,7 @@ async function readStream(stream: Readable): Promise { export class ExecProcess implements Result { protected _process?: ChildProcess; protected _aborted: boolean = false; + protected _killed: boolean = false; protected _options: Partial; protected _command: string; protected _args: readonly string[]; @@ -179,7 +183,7 @@ export class ExecProcess implements Result { } public get killed(): boolean { - return this._process?.killed === true; + return this._killed || this._process?.killed === true; } public pipe( @@ -318,10 +322,11 @@ export class ExecProcess implements Result { nodeOptions.env = computeEnv(cwd, nodeOptions.env, options.nodePath); + const killDescendants = options.killDescendants === true; const crossResult = normalizeSpawnCommand( this._command, this._args, - nodeOptions + killDescendants ? detachProcessGroup(nodeOptions) : nodeOptions ); const handle = spawn( @@ -330,6 +335,20 @@ export class ExecProcess implements Result { crossResult.options ); + if (killDescendants) { + // node's timeout and abort handling calls the public kill method + const kill = createKillFunction(handle); + handle.kill = (signal): boolean => { + const killed = kill(signal); + + if (killed) { + this._killed = true; + } + + return killed; + }; + } + if (handle.stderr) { this._streamErr = handle.stderr; } @@ -354,6 +373,7 @@ export class ExecProcess implements Result { protected _resetState(): void { this._aborted = false; + this._killed = false; this._processClosed = new Promise((resolve) => { this._resolveClose = resolve; }); diff --git a/src/test/kill-descendants_test.ts b/src/test/kill-descendants_test.ts new file mode 100644 index 0000000..cd76607 --- /dev/null +++ b/src/test/kill-descendants_test.ts @@ -0,0 +1,257 @@ +import {type ChildProcess} from 'node:child_process'; +import {once} from 'node:events'; +import os from 'node:os'; +import path from 'node:path'; +import {setTimeout} from 'node:timers/promises'; +import {describe, expect, onTestFinished, test, vi} from 'vitest'; +import {createKillFunction} from '../kill-descendants.js'; +import {x, type Options, type Result} from '../main.js'; + +const isWindows = os.platform() === 'win32'; +const fixture = path.join( + import.meta.dirname, + '../../test/fixtures/descendant.mjs' +); + +interface DescendantProcess { + subprocess: Result; + subprocessPid: number; + descendantPid: number; +} + +function isRunning(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +async function waitForExit(pid: number): Promise { + await vi.waitFor( + () => { + expect(isRunning(pid)).toBe(false); + }, + {timeout: 10000, interval: 100} + ); +} + +function forceKill(pid: number): void { + if (!isRunning(pid)) { + return; + } + + try { + process.kill(pid, 'SIGKILL'); + } catch { + // The process might have exited between the check and the signal. + } +} + +/** + * Starts the fixture, which spawns a descendant and writes its PID to stdout. + * Both processes are force killed once the calling test finishes, so a test + * which fails half way through cannot leave them behind. + */ +async function spawnDescendant( + options: Partial = {} +): Promise { + const subprocess = x('node', [fixture], options); + const subprocessPid = subprocess.pid; + const stdout = subprocess.process?.stdout; + + if (subprocessPid === undefined || stdout === null || stdout === undefined) { + throw new Error('Could not start subprocess'); + } + + const [chunk] = await once(stdout, 'data'); + const descendantPid = Number.parseInt(String(chunk), 10); + + if (!Number.isInteger(descendantPid)) { + throw new Error('Could not read descendant PID'); + } + + onTestFinished(() => { + forceKill(subprocessPid); + forceKill(descendantPid); + }); + + return {subprocess, subprocessPid, descendantPid}; +} + +describe('killDescendants', () => { + test('terminates descendants when kill() is called', async () => { + const {subprocess, subprocessPid, descendantPid} = await spawnDescendant({ + killDescendants: true + }); + + expect(isRunning(descendantPid)).toBe(true); + expect(subprocess.kill()).toBe(true); + await subprocess; + + expect(subprocess.killed).toBe(true); + expect(isRunning(subprocessPid)).toBe(false); + await waitForExit(descendantPid); + }); + + test('terminates descendants on timeout', async () => { + const {subprocess, subprocessPid, descendantPid} = await spawnDescendant({ + killDescendants: true, + timeout: 1000 + }); + + expect(isRunning(descendantPid)).toBe(true); + await expect(subprocess).rejects.toThrow(); + + expect(subprocess.killed).toBe(true); + expect(isRunning(subprocessPid)).toBe(false); + await waitForExit(descendantPid); + }); + + test('terminates descendants on abort', async () => { + const controller = new AbortController(); + const {subprocess, subprocessPid, descendantPid} = await spawnDescendant({ + killDescendants: true, + signal: controller.signal + }); + + expect(isRunning(descendantPid)).toBe(true); + controller.abort(); + await subprocess; + + expect(subprocess.aborted).toBe(true); + expect(subprocess.killed).toBe(true); + expect(isRunning(subprocessPid)).toBe(false); + await waitForExit(descendantPid); + }); + + test('terminates descendants alongside persist', async () => { + const {subprocess, subprocessPid, descendantPid} = await spawnDescendant({ + killDescendants: true, + persist: true + }); + + expect(subprocess.kill()).toBe(true); + await subprocess; + + expect(isRunning(subprocessPid)).toBe(false); + await waitForExit(descendantPid); + }); + + // Windows can already take descendants down with the direct child, so + // there is nothing to contrast the option against there. + if (!isWindows) { + test('leaves descendants running by default', async () => { + const {subprocess, subprocessPid, descendantPid} = + await spawnDescendant(); + + expect(subprocess.kill()).toBe(true); + await subprocess; + + expect(isRunning(subprocessPid)).toBe(false); + expect(isRunning(descendantPid)).toBe(true); + }); + + test('leaves descendants running on timeout by default', async () => { + const {subprocess, subprocessPid, descendantPid} = await spawnDescendant({ + timeout: 1000 + }); + + await expect(subprocess).rejects.toThrow(); + + expect(isRunning(subprocessPid)).toBe(false); + expect(isRunning(descendantPid)).toBe(true); + }); + + test('signals the process group with an explicit signal', async () => { + const {subprocess, subprocessPid, descendantPid} = await spawnDescendant({ + killDescendants: true + }); + + expect(subprocess.kill('SIGKILL')).toBe(true); + await subprocess; + + expect(isRunning(subprocessPid)).toBe(false); + await waitForExit(descendantPid); + }); + } + + test('signal 0 does not terminate anything', async () => { + const {subprocess, subprocessPid, descendantPid} = await spawnDescendant({ + killDescendants: true + }); + + expect(subprocess.kill(0)).toBe(true); + await setTimeout(100); + + expect(isRunning(subprocessPid)).toBe(true); + expect(isRunning(descendantPid)).toBe(true); + }); +}); + +if (isWindows) { + describe('taskkill', () => { + function createFakeSubprocess(): { + subprocess: ChildProcess; + directKill: ReturnType; + } { + const directKill = vi.fn(() => true); + return { + subprocess: {pid: 123, kill: directKill} as unknown as ChildProcess, + directKill + }; + } + + test('falls back to direct kill without a Windows directory', () => { + const {subprocess, directKill} = createFakeSubprocess(); + + expect(createKillFunction(subprocess, {})('SIGTERM')).toBe(true); + expect(directKill).toHaveBeenCalledWith('SIGTERM'); + }); + + test('falls back to direct kill for a relative Windows directory', () => { + const {subprocess, directKill} = createFakeSubprocess(); + const kill = createKillFunction(subprocess, {SystemRoot: 'Windows'}); + + expect(kill('SIGTERM')).toBe(true); + expect(directKill).toHaveBeenCalledWith('SIGTERM'); + }); + + test('falls back to direct kill for a UNC Windows directory', () => { + const {subprocess, directKill} = createFakeSubprocess(); + const kill = createKillFunction(subprocess, { + SystemRoot: '\\\\server\\share\\Windows' + }); + + expect(kill('SIGTERM')).toBe(true); + expect(directKill).toHaveBeenCalledWith('SIGTERM'); + }); + + test('falls back to direct kill when taskkill fails', async () => { + const {subprocess, directKill} = createFakeSubprocess(); + const kill = createKillFunction(subprocess, { + SystemRoot: 'C:\\MissingWindows' + }); + + expect(kill('SIGTERM')).toBe(true); + await vi.waitFor(() => { + expect(directKill).toHaveBeenCalledWith('SIGTERM'); + }); + }); + + test('runs taskkill when the Windows directory resolves', async () => { + const {subprocess, directKill} = createFakeSubprocess(); + const kill = createKillFunction(subprocess, process.env); + + expect(kill('SIGTERM')).toBe(true); + // An unresolved directory falls back synchronously, so a direct kill + // which only happens later proves taskkill was actually run. + expect(directKill).not.toHaveBeenCalled(); + // PID 123 does not exist, so taskkill reports a failure. + await vi.waitFor(() => { + expect(directKill).toHaveBeenCalledWith('SIGTERM'); + }); + }); + }); +} diff --git a/test/fixtures/descendant.mjs b/test/fixtures/descendant.mjs new file mode 100644 index 0000000..1c0b3b5 --- /dev/null +++ b/test/fixtures/descendant.mjs @@ -0,0 +1,16 @@ +import {spawn} from 'node:child_process'; +import path from 'node:path'; + +// Spawn a grandchild which outlives us, so tests can check whether killing +// this process took the whole tree with it. +const grandchild = path.join(import.meta.dirname, 'grandchild.mjs'); +const descendant = spawn(process.argv[0], [grandchild], { + stdio: 'ignore' +}); + +if (descendant.pid === undefined) { + throw new Error('Could not start descendant process'); +} + +process.stdout.write(String(descendant.pid)); +setTimeout(() => {}, 30000);