fix(core): kill child process tree in different running tasks#33636
fix(core): kill child process tree in different running tasks#33636FrozenPandaz merged 1 commit intomasterfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit 2ec248d
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
These changes fix the test timeouts by calling treeKill directly in signal handlers instead of the async this.kill() method. Signal handlers cannot await promises, so the previous approach left the async kill operations incomplete during test cleanup, causing Storybook processes to remain in an inconsistent state and tests to timeout waiting for proper termination.
We could not verify this fix.
Suggested Fix changes
diff --git a/packages/nx/src/executors/run-commands/running-tasks.ts b/packages/nx/src/executors/run-commands/running-tasks.ts
index b6ea2be8b9..8dbeaf7fea 100644
--- a/packages/nx/src/executors/run-commands/running-tasks.ts
+++ b/packages/nx/src/executors/run-commands/running-tasks.ts
@@ -515,20 +515,40 @@ class RunningNodeProcess implements RunningTask {
});
// Terminate any task processes on exit
process.on('exit', () => {
- this.kill();
+ // Call tree-kill without callback (fire-and-forget) since exit handlers can't be async
+ if (this.childProcess?.pid) {
+ treeKill(this.childProcess.pid, (err) => {
+ // Callback for tree-kill, but in exit handler it may not complete
+ });
+ }
});
process.on('SIGINT', () => {
- this.kill('SIGTERM');
+ // Call tree-kill without waiting for completion
+ if (this.childProcess?.pid) {
+ treeKill(this.childProcess.pid, 'SIGTERM', (err) => {
+ // Ignore errors - process may have already exited
+ });
+ }
// we exit here because we don't need to write anything to cache.
process.exit(signalToCode('SIGINT'));
});
process.on('SIGTERM', () => {
- this.kill('SIGTERM');
+ // Call tree-kill without waiting for completion
+ if (this.childProcess?.pid) {
+ treeKill(this.childProcess.pid, 'SIGTERM', (err) => {
+ // Ignore errors - process may have already exited
+ });
+ }
// no exit here because we expect child processes to terminate which
// will store results to the cache and will terminate this process
});
process.on('SIGHUP', () => {
- this.kill('SIGTERM');
+ // Call tree-kill without waiting for completion
+ if (this.childProcess?.pid) {
+ treeKill(this.childProcess.pid, 'SIGTERM', (err) => {
+ // Ignore errors - process may have already exited
+ });
+ }
// no exit here because we expect child processes to terminate which
// will store results to the cache and will terminate this process
});
Or Apply changes locally with:
npx nx-cloud apply-locally UmET-HR4v
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
🐳 We have a release for that!This PR has a release associated with it. You can try it out using this command: npx create-nx-workspace@0.0.0-pr-33636-5c093fd my-workspaceOr just copy this version and use it in your own command: 0.0.0-pr-33636-5c093fd
To request a new release for this pull request, mention someone from the Nx team or the |
5c093fd to
5a5f5c3
Compare
5a5f5c3 to
dccd2f2
Compare
dccd2f2 to
2ec248d
Compare
## 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)
## 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)
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |

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 usingchildProcess.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.
Changes
RunningNodeProcessto usethis.kill()instead ofthis.childProcess.kill(), leveraging the existingtree-killimplementationtree-killtoNodeChildProcessWithNonDirectOutputandNodeChildProcessWithDirectOutputkill methods to ensure entire process trees are terminated