Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions packages/cli/src/utils/__tests__/command.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,34 @@ describe('command runners', () => {
).rejects.toThrow(/timed out after 200ms/);
});

it(
'times out even when a grandchild inherits the stdio pipes',
{ timeout: 10_000 },
async () => {
// Arbitrary project code run by a config worker can spawn its own
// children. This child spawns a grandchild that inherits the piped
// stdio, then wedges like a blocking plugin factory. Without a tree
// kill the SIGKILL reaches only the direct child, the grandchild
// keeps the stdout pipe open, `close` never fires, and the promise
// never settles. The grandchild self-terminates after 15s so a
// failing run leaves nothing behind.
await expect(
runCommandSilently({
command: process.execPath,
args: [
'-e',
`const { spawn } = require('node:child_process');
spawn(process.execPath, ['-e', 'setTimeout(() => {}, 15_000)'], { stdio: 'inherit' });
setInterval(() => {}, 1000);`,
],
cwd: process.cwd(),
envs: process.env,
timeoutMs: 500,
}),
).rejects.toThrow(/timed out after 500ms/);
},
);

it('does not reject a fast child because a timeout is configured', async () => {
const result = await runCommandSilently({
command: process.execPath,
Expand Down
19 changes: 18 additions & 1 deletion packages/cli/src/utils/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ export async function runCommandSilently(options: RunCommandOptions): Promise<Ru
stdio: ['ignore', 'pipe', 'pipe'],
cwd: options.cwd,
env: options.envs,
// Own process group (POSIX) so the timeout can kill the whole tree: the
// child runs arbitrary project code that may spawn its own children.
detached: process.platform !== 'win32',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Detach only commands that have a timeout

On POSIX this places every silent command in a new session, including the no-timeout install, format, and migration commands. Consequently, Ctrl-C signals the foreground process group containing vp but not the detached child; because this runner has no shutdown handler that forwards the signal, a package installation or build script can continue modifying the project after the user cancels the CLI. Limit detached to timeout-bearing invocations (or explicitly terminate the child during parent shutdown) so ordinary commands retain signal propagation.

Useful? React with 👍 / 👎.

});
const promise = new Promise<RunCommandResult>((resolve, reject) => {
const stdout: Buffer[] = [];
Expand All @@ -51,7 +54,21 @@ export async function runCommandSilently(options: RunCommandOptions): Promise<Ru
? undefined
: setTimeout(() => {
timedOut = true;
child.kill('SIGKILL');
if (process.platform !== 'win32' && child.pid) {
try {
process.kill(-child.pid, 'SIGKILL');
} catch {
child.kill('SIGKILL');
}
} else {
child.kill('SIGKILL');
}
// A descendant that inherited the pipes can hold them open past
// the kill (a Windows child tree, or a POSIX process that left
// the group). Release our ends so `close` always fires; the
// timeout path rejects without reading the output anyway.
child.stdout?.destroy();
child.stderr?.destroy();
}, options.timeoutMs);
timer?.unref();
child.stdout?.on('data', (data) => {
Expand Down
Loading