Skip to content

fix(core): kill child process tree in different running tasks#33636

Merged
FrozenPandaz merged 1 commit intomasterfrom
core/kill-child-process-tree
Nov 27, 2025
Merged

fix(core): kill child process tree in different running tasks#33636
FrozenPandaz merged 1 commit intomasterfrom
core/kill-child-process-tree

Conversation

@leosvelperez
Copy link
Copy Markdown
Member

@leosvelperez leosvelperez commented Nov 27, 2025

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.

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

@leosvelperez leosvelperez self-assigned this Nov 27, 2025
@leosvelperez leosvelperez requested a review from a team as a code owner November 27, 2025 09:46
@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 27, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
nx-dev Ready Ready Preview Nov 27, 2025 1:22pm

@netlify
Copy link
Copy Markdown

netlify Bot commented Nov 27, 2025

Deploy Preview for nx-docs ready!

Name Link
🔨 Latest commit 2ec248d
🔍 Latest deploy log https://app.netlify.com/projects/nx-docs/deploys/69284f9289443600083a6ce8
😎 Deploy Preview https://deploy-preview-33636--nx-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@nx-cloud
Copy link
Copy Markdown
Contributor

nx-cloud Bot commented Nov 27, 2025

View your CI Pipeline Execution ↗ for commit 2ec248d

Command Status Duration Result
nx affected --targets=lint,test,test-kt,build,e... ✅ Succeeded 8m 10s View ↗
nx run-many -t check-imports check-lock-files c... ✅ Succeeded 2m 46s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 12s View ↗
nx-cloud record -- nx format:check ✅ Succeeded 2s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded <1s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-27 13:30:11 UTC

nx-cloud[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@nx-cloud nx-cloud Bot left a comment

Choose a reason for hiding this comment

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

Important

A new CI pipeline execution was requested that may update the conclusion below...

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
     });

Apply fix via Nx Cloud  Reject fix via Nx Cloud


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

@github-actions
Copy link
Copy Markdown
Contributor

🐳 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-workspace

Or just copy this version and use it in your own command:

0.0.0-pr-33636-5c093fd
Release details 📑
Published version 0.0.0-pr-33636-5c093fd
Triggered by @leosvelperez
Branch core/kill-child-process-tree
Commit 5c093fd
Workflow run 19733385638

To request a new release for this pull request, mention someone from the Nx team or the @nrwl/nx-pipelines-reviewers.

@leosvelperez leosvelperez force-pushed the core/kill-child-process-tree branch from dccd2f2 to 2ec248d Compare November 27, 2025 13:18
@leosvelperez leosvelperez marked this pull request as ready for review November 27, 2025 13:30
@FrozenPandaz FrozenPandaz merged commit 2178340 into master Nov 27, 2025
19 checks passed
@FrozenPandaz FrozenPandaz deleted the core/kill-child-process-tree branch November 27, 2025 16:53
FrozenPandaz pushed a commit that referenced this pull request Nov 27, 2025
## 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)
FrozenPandaz pushed a commit that referenced this pull request Nov 27, 2025
## 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)
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 3, 2025

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.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Dec 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants