Potentially incorrect signal sent to run-executor during shutdown

[LonguCodes]

Current Behavior

When running any command and sending a signal to it, .bin/nx sends different signals to the underlying process (run-executor) depending on the source of the signal

If using CTRL+C to send SIGINT signal, both the main process and the underlying process receive SIGINT (not sure why)

If using kill -s INT <pid of the main process>, the main process gets SIGINT, but the child process (run-executor) gets SIGTERM

This manifests in the underlying application receiving incorrect signals and/or @nx/js:node not passing the signal to the app process

Expected Behavior

In both situations, SIGINT should be received by the run-executor process

GitHub Repo

No response

Steps to Reproduce

1. Install Nx
2. Add

process.on('SIGINT', () => {
  console.log('Got SIGINT');
  process.exit();
});
process.on('SIGTERM', () => {
  console.log('Got SIGTERM');
  process.exit();
});

to the compiled nx library in node_modules/nx/bin/run-executor.js 3. Run any command, that runs indefinitely 4. kill the process using CTRL-C 5. Run the command again 6. kill the process using kill -s INT <pid>

Nx Report

Node   : 18.14.0
   OS     : linux-x64
   yarn   : 1.22.19
   
   nx                 : 16.5.2
   @nx/js             : 16.5.2
   @nx/jest           : 16.5.2
   @nx/linter         : 16.5.2
   @nx/workspace      : 16.5.2
   @nx/cypress        : 16.5.2
   @nx/devkit         : 16.5.2
   @nx/esbuild        : 16.5.2
   @nx/eslint-plugin  : 16.5.2
   @nx/nest           : 16.5.2
   @nx/node           : 16.5.2
   @nx/react          : 16.5.2
   @nrwl/tao          : 16.5.2
   @nx/vite           : 16.5.2
   @nx/web            : 16.5.2
   @nx/webpack        : 16.5.2
   typescript         : 5.1.6

Failure Logs

No response

Operating System

• [ ] macOS
• [X] Linux
• [ ] Windows
• [ ] Other (Please specify)

Additional Information

After discussion, I'm willing to create a PR to fix mentioned issue

[LonguCodes]

Found the potentially incorrect lines https://github.com/nrwl/nx/blob/21d3c5e63c2e41b1f414d01194fbdb274a3185b2/packages/nx/src/tasks-runner/forked-process-task-runner.ts#L480-L506

[simondotm]

My plugin for firebase has experienced this issue for a while now; the firebase emulation suite is not terminating cleanly when invoked using nx:run-command.

The firebase cli emulator command is a long running task and the problem is that the firebase cli tool does not receive a SIGINT for a ctrl+c exit so it does not exit cleanly.

In the above code referenced by @LonguCodes , my specific problem is fixed by simply not calling process.exit() in the SIGINT handler.

Questions:

1. Why does the SIGINT handler kill with SIGTERM instead?
2. Is it absolutely necessary to process.exit() in SIGINT ? (since not doing that for ~~SIGKILL~~ EDIT: SIGTERM)

[LonguCodes]

@simondotm If you do not use process.exit() in the handler of SIGINT or SIGTERM, the process will not shut down. This would mean that we would kill every child process, but not the main one.

SIGKILL is uncatchable, so you are not able to do process.on('SIGKILL')

EDIT:

I think you meant SIGTERM, not SIGKILL. Not sure about the answer, will comment when I find out

[simondotm]

Sorry yes, I meant SIGTERM in question 2

[github-actions[bot]]

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs. If we missed this issue please reply to keep it active. Thanks for being a part of the Nx community! 🙏

[simondotm]

👀

[simondotm]

Since updating from Nx 16 to Nx 17 I see different behaviour from CTRL+C now, tasks spawned via nx CLI seem to be aggressively killed and I see: ELIFECYCLE  Command failed with exit code 128.

This happens in Nx 18 for me also.

And now my previous workaround of creating a custom serve executor that spawned its own processes and handled exit signals itself to ensure my spawned processes would receive a SIGINT, no longer works - presumably because the top level process is nx serve and my executor is a child process of that.

I see the forked-process-task-runner.js module has changed quite a bit since I last looked at it, but I've not looked any further into the code to understand if/why child processes are not getting the exit signals they need.

This is really unhelpful for my plugin as the Firebase emulator really needs to receive that exit event to shut itself down gracefully.

[simondotm]

Linking to #13766 for reference