fix(core): kill discrete task processes during SIGINT cleanup#35175
Draft
FrozenPandaz wants to merge 6 commits intomasterfrom
Draft
fix(core): kill discrete task processes during SIGINT cleanup#35175FrozenPandaz wants to merge 6 commits intomasterfrom
FrozenPandaz wants to merge 6 commits intomasterfrom
Conversation
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
View your CI Pipeline Execution ↗ for commit ae85d03
☁️ Nx Cloud last updated this comment at |
BatchProcess.kill() was only sending the signal to the immediate child process (the Node fork worker), leaving grandchild processes alive. This is especially problematic for Gradle where the process tree is deep (Node fork → Java JVM → Gradle Daemon → Gradle Workers).
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
execSync blocks the Node event loop, preventing the batch worker from responding to signals or IPC messages during SIGINT cleanup. Switch to spawn so the process can be properly terminated.
Contributor
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We updated the SIGINT e2e test helpers to add detached: true and send signals via process.kill(-nxProcess.pid!, signal) targeting the entire process group. This ensures the signal reaches the nx process even when it runs as a grandchild of the shell, fixing the consistent timeout failures where exitedCleanly was always false.
Note
⏳ We are verifying this fix by re-running e2e-nx:e2e-ci--src/run.test.ts.
Suggested Fix changes
diff --git a/e2e/nx/src/run.test.ts b/e2e/nx/src/run.test.ts
index 970cf293f5..132b97438e 100644
--- a/e2e/nx/src/run.test.ts
+++ b/e2e/nx/src/run.test.ts
@@ -946,6 +946,9 @@ describe('Nx Running Tests', () => {
},
shell: true,
stdio: 'pipe',
+ // detached creates a new process group so we can send signals to
+ // the entire group (shell + nx + its children) via -pid
+ detached: true,
});
// Wait for the child process to start.
@@ -987,14 +990,17 @@ describe('Nx Running Tests', () => {
expect(childPid).toBeDefined();
expect(isProcessAlive(childPid)).toBe(true);
- // Send SIGINT
- nxProcess.kill('SIGINT');
+ // Send SIGINT to the entire process group so the signal reaches
+ // the nx process even when it is a grandchild of the shell.
+ process.kill(-nxProcess.pid!, 'SIGINT');
// Nx should exit promptly (cleanup should not hang).
// Without the fix, cleanup hangs waiting for unkilled discrete tasks.
const exitedCleanly = await new Promise<boolean>((resolve) => {
const timeout = setTimeout(() => {
- nxProcess.kill('SIGKILL');
+ try {
+ process.kill(-nxProcess.pid!, 'SIGKILL');
+ } catch {}
resolve(false);
}, 10_000);
nxProcess.on('exit', () => {
@@ -1037,6 +1043,7 @@ describe('Nx Running Tests', () => {
},
shell: true,
stdio: 'pipe',
+ detached: true,
});
// Poll for PID file (TUI swallows stdout so we can't parse it)
@@ -1051,20 +1058,25 @@ describe('Nx Running Tests', () => {
}
if (!childPid) {
- nxProcess.kill('SIGKILL');
+ try {
+ process.kill(-nxProcess.pid!, 'SIGKILL');
+ } catch {}
throw new Error('Timed out waiting for PID file from TUI task');
}
expect(isProcessAlive(childPid)).toBe(true);
- // Send SIGINT
- nxProcess.kill('SIGINT');
+ // Send SIGINT to the entire process group so the signal reaches
+ // the nx process even when it is a grandchild of the shell.
+ process.kill(-nxProcess.pid!, 'SIGINT');
// Nx should exit promptly — if cleanup hangs because discrete tasks
// aren't killed, this will time out.
const exitedCleanly = await new Promise<boolean>((resolve) => {
const timeout = setTimeout(() => {
- nxProcess.kill('SIGKILL');
+ try {
+ process.kill(-nxProcess.pid!, 'SIGKILL');
+ } catch {}
resolve(false);
}, 10_000);
nxProcess.on('exit', () => {
🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.
Or Apply changes locally with:
npx nx-cloud apply-locally cErl-2Ff9
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current Behavior
When Nx receives SIGINT (Ctrl+C),
performCleanup()in the task orchestrator kills continuous tasks and run-commands tasks, but not discrete tasks (executor-based builds like@nx/js:tsc,nx:run-script, etc.).In TUI mode, fork workers run in separate PTY process groups and don't receive SIGINT directly. Without explicit cleanup,
performCleanup()hangs atawait Promise.all(this.discreteTaskExitHandled.values())waiting for tasks that will never exit.Expected Behavior
All task types — discrete, continuous, and run-commands — should be explicitly killed during SIGINT cleanup so the process exits promptly.
Related Issue(s)
N/A — found via code inspection and confirmed with tests.
Notes