Skip to content

Commit 7fc161e

Browse files
leosvelperezFrozenPandaz
authored andcommitted
fix(core): kill child process tree in different running tasks (#33636)
## Current Behavior When Nx commands finish or receive termination signals (SIGINT, SIGTERM, SIGHUP), child processes spawned by continuous tasks (such as `nx serve`) can remain orphaned in certain scenarios. This happens because only the direct child process is killed using `childProcess.kill()`, leaving grandchild processes running. ## Expected Behavior When Nx terminates, all processes in the spawned process tree should be properly terminated and no orphaned processes should remain. ## Related Issue(s) Fixes #32438 Fixes #33460 ## Changes - Updated signal handlers in `RunningNodeProcess` to use `this.kill()` instead of `this.childProcess.kill()`, leveraging the existing `tree-kill` implementation - Added `tree-kill` to `NodeChildProcessWithNonDirectOutput` and `NodeChildProcessWithDirectOutput` kill methods to ensure entire process trees are terminated (cherry picked from commit 2178340)
1 parent 4e6aa05 commit 7fc161e

2 files changed

Lines changed: 17 additions & 12 deletions

File tree

packages/nx/src/executors/run-commands/running-tasks.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -486,20 +486,20 @@ class RunningNodeProcess implements RunningTask {
486486
});
487487
// Terminate any task processes on exit
488488
process.on('exit', () => {
489-
this.childProcess.kill();
489+
this.kill();
490490
});
491491
process.on('SIGINT', () => {
492-
this.childProcess.kill('SIGTERM');
492+
this.kill('SIGTERM');
493493
// we exit here because we don't need to write anything to cache.
494494
process.exit(signalToCode('SIGINT'));
495495
});
496496
process.on('SIGTERM', () => {
497-
this.childProcess.kill('SIGTERM');
497+
this.kill('SIGTERM');
498498
// no exit here because we expect child processes to terminate which
499499
// will store results to the cache and will terminate this process
500500
});
501501
process.on('SIGHUP', () => {
502-
this.childProcess.kill('SIGTERM');
502+
this.kill('SIGTERM');
503503
// no exit here because we expect child processes to terminate which
504504
// will store results to the cache and will terminate this process
505505
});

packages/nx/src/tasks-runner/running-tasks/node-child-process.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
import { ChildProcess, Serializable } from 'child_process';
2-
import { signalToCode } from '../../utils/exit-codes';
3-
import { RunningTask } from './running-task';
4-
import { Transform } from 'stream';
51
import * as chalk from 'chalk';
2+
import type { ChildProcess, Serializable } from 'child_process';
63
import { readFileSync } from 'fs';
4+
import { Transform } from 'stream';
5+
import * as treeKill from 'tree-kill';
6+
import { signalToCode } from '../../utils/exit-codes';
7+
import type { RunningTask } from './running-task';
78

89
export class NodeChildProcessWithNonDirectOutput implements RunningTask {
910
private terminalOutput: string = '';
@@ -99,8 +100,10 @@ export class NodeChildProcessWithNonDirectOutput implements RunningTask {
99100
}
100101
}
101102
public kill(signal?: NodeJS.Signals) {
102-
if (this.childProcess.connected) {
103-
this.childProcess.kill(signal);
103+
if (this.childProcess?.pid) {
104+
treeKill(this.childProcess.pid, signal, () => {
105+
// Ignore errors - process may have already exited
106+
});
104107
}
105108
}
106109
}
@@ -222,8 +225,10 @@ export class NodeChildProcessWithDirectOutput implements RunningTask {
222225
}
223226

224227
kill(signal?: NodeJS.Signals): void {
225-
if (this.childProcess.connected) {
226-
this.childProcess.kill(signal);
228+
if (this.childProcess?.pid) {
229+
treeKill(this.childProcess.pid, signal, () => {
230+
// Ignore errors - process may have already exited
231+
});
227232
}
228233
}
229234
}

0 commit comments

Comments
 (0)