Fix subprocesses hanging after exceeding maxBuffer

Closing the output stream does not stop a subprocess that stays alive without writing again, so Execa's promise never settled. The subprocess is now terminated with the `killSignal` option when the limit is exceeded, like with synchronous methods and the other termination options. The error reports the real exit of the subprocess, which might have exited on its own before being terminated.

Author: sindresorhus

docs/api.md

@@ -1035,7 +1035,7 @@ By default, this applies to both `stdout` and `stderr`, but [different values ca
 
 With [synchronous methods](execution.md#synchronous-execution), this cannot be enforced per file descriptor while the subprocess runs, since `spawnSync()` only accepts a single value, which limits the total output of all file descriptors. The `maxBuffer.stdout` value is used for it. The other file descriptors' own values are enforced from the output, once the subprocess ended, and cannot be higher than `maxBuffer.stdout`. Also, with synchronous methods, this is measured in bytes instead of [characters or lines](output.md#big-output), for the same reason.
 
-When reached, [`error.isMaxBuffer`](#errorismaxbuffer) becomes `true`.
+When reached, [`error.isMaxBuffer`](#errorismaxbuffer) becomes `true` and the subprocess is terminated with the [`killSignal`](#optionskillsignal) option.
 
 [More info.](output.md#big-output)
 

docs/errors.md

@@ -56,7 +56,7 @@ The subprocess can fail for other reasons. Some of them can be detected using a
 - [`error.isCanceled`](api.md#erroriscanceled): [`cancelSignal`](termination.md#canceling) option.
 - [`error.isGracefullyCanceled`](api.md#errorisgracefullycanceled): `cancelSignal` option, if the [`gracefulCancel`](termination.md#graceful-termination) option is `true`.
 - [`error.isMaxBuffer`](api.md#errorismaxbuffer): [`maxBuffer`](output.md#big-output) option.
-- [`error.isTerminated`](api.md#erroristerminated): [signal termination](termination.md#signal-termination). This includes the [`timeout`](termination.md#timeout) and [`forceKillAfterDelay`](termination.md#forceful-termination) options since those terminate the subprocess with a [signal](termination.md#default-signal). This also includes the [`cancelSignal`](termination.md#canceling) option unless the [`gracefulCancel`](termination.md#graceful-termination) option is `true`. This does not include the [`maxBuffer`](output.md#big-output) option.
+- [`error.isTerminated`](api.md#erroristerminated): [signal termination](termination.md#signal-termination). This includes the [`timeout`](termination.md#timeout) and [`forceKillAfterDelay`](termination.md#forceful-termination) options since those terminate the subprocess with a [signal](termination.md#default-signal). This also includes the [`cancelSignal`](termination.md#canceling) option unless the [`gracefulCancel`](termination.md#graceful-termination) option is `true`, and the [`maxBuffer`](output.md#big-output) option, since reaching the limit terminates the subprocess with the [`killSignal`](api.md#optionskillsignal) option, unless the subprocess exits on its own, for example by handling `killSignal`. With [synchronous methods](execution.md#synchronous-execution), the subprocess is only terminated when its total output exceeds `maxBuffer.stdout`.
 
 Otherwise, the subprocess failed because either:
 - An exception was thrown in a [stream](streams.md) or [transform](transform.md).

docs/output.md

@@ -171,7 +171,7 @@ await execa({stdin: 'ignore', stdout: 'ignore', stderr: 'ignore'})`npm run build
 
 To prevent high memory consumption, a maximum output size can be set using the [`maxBuffer`](api.md#optionsmaxbuffer) option. It defaults to 100MB.
 
-When this threshold is hit, the subprocess fails and [`error.isMaxBuffer`](api.md#errorismaxbuffer) becomes `true`. The truncated output is still available using [`error.stdout`](api.md#resultstdout), [`error.stderr`](api.md#resultstderr) and [`error.ipcOutput`](api.md#resultipcoutput).
+When this threshold is hit, [`error.isMaxBuffer`](api.md#errorismaxbuffer) becomes `true` and the subprocess is terminated with the [`killSignal`](api.md#optionskillsignal) option. The truncated output is still available using [`error.stdout`](api.md#resultstdout), [`error.stderr`](api.md#resultstderr) and [`error.ipcOutput`](api.md#resultipcoutput).
 
 This is measured:
 - By default: in [characters](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length).

lib/resolve/wait-subprocess.js

@@ -1,5 +1,6 @@
 import {once} from 'node:events';
 import {isStream as isNodeStream} from 'is-stream';
+import {MaxBufferError} from 'get-stream';
 import {throwOnTimeout} from '../terminate/timeout.js';
 import {throwOnCancel} from '../terminate/cancel.js';
 import {throwOnGracefulCancel} from '../terminate/graceful.js';
@@ -114,6 +115,11 @@ export const waitForSubprocessResult = async ({
 		]);
 	} catch (error) {
 		context.terminationReason ??= 'other';
+		// Closing the output stream does not stop a subprocess that stays alive without writing again.
+		if (error instanceof MaxBufferError) {
+			kill();
+		}
+
 		return Promise.all([
 			{error},
 			exitPromise,

test/fixtures/max-buffer-forever.js

@@ -4,5 +4,15 @@ import {getWriteStream} from '../helpers/fs.js';
 
 const fdNumber = Number(process.argv[2]);
 const bytes = '.'.repeat(Number(process.argv[3]));
+// The signal is ignored, or makes the subprocess exit with a specific exit code
+const [signal, exitCode] = process.argv.slice(4);
+if (signal !== undefined) {
+	process.on(signal, () => {
+		if (exitCode !== undefined) {
+			process.exit(Number(exitCode));
+		}
+	});
+}
+
 getWriteStream(fdNumber).write(bytes);
 setTimeout(() => {}, 1e8);

test/io/max-buffer.js

@@ -1,5 +1,6 @@
 import assert from 'node:assert/strict';
 import {Buffer} from 'node:buffer';
+import {platform} from 'node:process';
 import {readFile, rm} from 'node:fs/promises';
 import test from 'node:test';
 import getStream from 'get-stream';
@@ -17,6 +18,97 @@ setFixtureDirectory();
 const maxBufferMessage = {message: /maxBuffer exceeded/};
 const maxBufferCodeSync = {code: 'ENOBUFS'};
 
+// Bound the regression even when exceeding maxBuffer does not terminate the subprocess.
+const assertMaxBufferRejects = async subprocess => {
+	const cleanup = setTimeout(() => {
+		subprocess.kill('SIGKILL');
+	}, 5000);
+	try {
+		const error = await assertRejects(subprocess);
+		assert.equal(error.isMaxBuffer, true);
+		return error;
+	} finally {
+		clearTimeout(cleanup);
+	}
+};
+
+const assertMaxBufferTermination = async (subprocess, expectedSignal = 'SIGTERM') => {
+	const error = await assertMaxBufferRejects(subprocess);
+	assert.equal(error.isTerminated, true);
+	assert.equal(error.signal, expectedSignal);
+	assert.equal(error.exitCode, undefined);
+	assert.equal(error.timedOut, false);
+	return error;
+};
+
+const testAliveMaxBuffer = async (fdNumber, options = {}) => {
+	const error = await assertMaxBufferTermination(execa('max-buffer-forever.js', [`${fdNumber}`, '100'], {...fullStdio, maxBuffer: 5, ...options}), options.killSignal);
+	assert.equal(error.stdio[fdNumber], '.....');
+};
+
+test('maxBuffer terminates a subprocess that stays alive without more output', () => testAliveMaxBuffer(1));
+test('maxBuffer terminates a subprocess that stays alive without more stderr', () => testAliveMaxBuffer(2));
+test('maxBuffer terminates a subprocess that stays alive without more stdio[3]', () => testAliveMaxBuffer(3));
+test('maxBuffer termination uses killSignal', () => testAliveMaxBuffer(1, {killSignal: 'SIGINT'}));
+
+// Windows terminates immediately instead of delivering catchable signals.
+if (platform !== 'win32') {
+	test('maxBuffer termination forcefully kills a subprocess ignoring killSignal', async () => {
+		const error = await assertMaxBufferTermination(execa('max-buffer-forever.js', ['1', '100', 'SIGINT'], {
+			maxBuffer: 5,
+			killSignal: 'SIGINT',
+			forceKillAfterDelay: 50,
+		}), 'SIGKILL');
+		assert.equal(error.isForcefullyTerminated, true);
+		assert.equal(error.stdout, '.....');
+	});
+
+	test('maxBuffer termination reports the exit code of a subprocess handling killSignal', async () => {
+		const error = await assertMaxBufferRejects(execa('max-buffer-forever.js', ['1', '100', 'SIGTERM', '5'], {maxBuffer: 5}));
+		assert.equal(error.isTerminated, false);
+		assert.equal(error.signal, undefined);
+		assert.equal(error.exitCode, 5);
+		assert.equal(error.stdout, '.....');
+	});
+}
+
+test('maxBuffer termination applies to lines', async () => {
+	const error = await assertMaxBufferTermination(execa('max-buffer-forever.js', ['1', '100'], {
+		maxBuffer: 1,
+		lines: true,
+		stdout: {
+			* transform() {
+				yield 'first\nsecond\n';
+			},
+			binary: true,
+		},
+	}));
+	assert.deepEqual(error.stdout, ['first']);
+	assertErrorMessage(error.shortMessage, {length: 1, unit: 'lines'});
+});
+
+test('maxBuffer termination applies to object mode', async () => {
+	const error = await assertMaxBufferTermination(execa('max-buffer-forever.js', ['1', '100'], {
+		maxBuffer: 1,
+		stdout: {
+			* transform() {
+				yield {index: 1};
+				yield {index: 2};
+			},
+			binary: true,
+			objectMode: true,
+		},
+	}));
+	assert.deepEqual(error.stdout, [{index: 1}]);
+	assertErrorMessage(error.shortMessage, {length: 1, unit: 'objects'});
+});
+
+test('maxBuffer termination applies to IPC messages', async () => {
+	const error = await assertMaxBufferTermination(execa('ipc-send-forever.js', {ipc: true, maxBuffer: {ipc: 0}}));
+	assert.deepEqual(error.ipcOutput, []);
+	assert.ok(error.shortMessage.includes('IPC output was larger than 0 messages'));
+});
+
 const runMaxBuffer = async (execaMethod, fdNumber, options) => {
 	const error = execaMethod === execa
 		? await assertRejects(getMaxBufferSubprocess(execaMethod, fdNumber, options), maxBufferMessage)
@@ -47,20 +139,31 @@ test('maxBuffer does not affect stderr if too high, sync', () => testMaxBufferSu
 test('maxBuffer does not affect stdio[*] if too high, sync', () => testMaxBufferSuccess(execaSync, 3, false));
 test('maxBuffer does not affect all if too high, sync', () => testMaxBufferSuccess(execaSync, 1, true));
 
-const testGracefulExit = async (fixtureName, expectedExitCode) => {
-	const {isMaxBuffer, shortMessage, exitCode, signal, stdout} = await assertRejects(
+// The subprocess might exit on its own before it is terminated, in which case its real exit is reported
+const assertMaxBufferExit = ({exitCode, signal, isTerminated}, expectedExitCode) => {
+	if (signal === undefined) {
+		assert.equal(isTerminated, false);
+		assert.equal(exitCode, expectedExitCode);
+	} else {
+		assert.equal(isTerminated, true);
+		assert.equal(signal, 'SIGTERM');
+		assert.equal(exitCode, undefined);
+	}
+};
+
+const testMaxBufferExit = async (fixtureName, expectedExitCode) => {
+	const error = await assertRejects(
 		execa(fixtureName, ['1', '.'.repeat(maxBuffer + 1)], {maxBuffer}),
 		maxBufferMessage,
 	);
-	assert.equal(isMaxBuffer, true);
-	assertErrorMessage(shortMessage);
-	assert.equal(exitCode, expectedExitCode);
-	assert.equal(signal, undefined);
-	assert.equal(stdout, getExpectedOutput());
+	assert.equal(error.isMaxBuffer, true);
+	assertMaxBufferExit(error, expectedExitCode);
+	assertErrorMessage(error.shortMessage);
+	assert.equal(error.stdout, getExpectedOutput());
 };
 
-test('maxBuffer terminates stream gracefully, more writes', () => testGracefulExit('noop-repeat.js', 1));
-test('maxBuffer terminates stream gracefully, no more writes', () => testGracefulExit('noop-fd.js', 0));
+test('maxBuffer terminates subprocess, more writes', () => testMaxBufferExit('noop-repeat.js', 1));
+test('maxBuffer terminates subprocess, no more writes', () => testMaxBufferExit('noop-fd.js', 0));
 
 const testGracefulExitSync = fixtureName => {
 	const {isMaxBuffer, shortMessage, exitCode, signal, stdout} = assertThrows(() => {
@@ -173,6 +276,8 @@ test('maxBuffer.stderr is used when lower than maxBuffer.stdout, sync', () => {
 	assertErrorMessage(error.shortMessage, {execaMethod: execaSync, fdNumber: 2, length});
 	assert.equal(error.stdout, getExpectedOutput(maxBuffer + 1));
 	assert.equal(error.stderr, getExpectedOutput(length));
+	// Unlike asynchronous methods, the subprocess is not terminated, since it already exited
+	assert.equal(error.isTerminated, false);
 	assert.equal(error.signal, undefined);
 	assert.equal(error.exitCode, 0);
 });
@@ -184,8 +289,7 @@ test('maxBuffer.stderr is used when lower than maxBuffer.stdout', async () => {
 	assertErrorMessage(error.shortMessage, {fdNumber: 2, length});
 	assert.equal(error.stdout, getExpectedOutput(maxBuffer + 1));
 	assert.equal(error.stderr, getExpectedOutput(length));
-	assert.equal(error.signal, undefined);
-	assert.equal(error.exitCode, 0);
+	assertMaxBufferExit(error, 0);
 });
 
 test('maxBuffer is not hit when under each file descriptor\'s value, sync', () => {

test/terminate/kill-force.js

@@ -146,7 +146,6 @@ if (isWindows) {
 		const subprocess = execa('noop-forever.js', ['.'], {...noKillableSimpleOptions, maxBuffer: 1});
 		const [chunk] = await once(subprocess.stdout, 'data');
 		assert.equal(chunk.toString(), '.\n');
-		subprocess.kill();
 		const {isTerminated, signal, isForcefullyTerminated, shortMessage} = await assertRejects(subprocess);
 		assert.equal(isTerminated, true);
 		assert.equal(signal, 'SIGKILL');

types/arguments/options.d.ts

@@ -194,7 +194,7 @@ export type CommonOptions<
 
 	With synchronous methods, this cannot be enforced per file descriptor while the subprocess runs, since `spawnSync()` only accepts a single value, which limits the total output of all file descriptors. The `maxBuffer.stdout` value is used for it. The other file descriptors' own values are enforced from the output, once the subprocess ended, and cannot be higher than `maxBuffer.stdout`. Also, with synchronous methods, this is measured in bytes instead of characters or lines, for the same reason.
 
-	When reached, `error.isMaxBuffer` becomes `true`.
+	When reached, `error.isMaxBuffer` becomes `true` and the subprocess is terminated with the `killSignal` option.
 
 	@default 100_000_000
 	*/