fix(core): send signals when killing child process on unix (#30987)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
rspack and next do not get killed when using the tui. This appears to be
caused by them not responding correctly to SIGTERM, so we need a way to
pass signals with our `kill` call.

## Expected Behavior
We properly shutdown rspack and next servers

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #

Cargo.lock

@@ -2032,6 +2032,18 @@ dependencies = [
  "libc",
 ]
 
+[[package]]
+name = "nix"
+version = "0.30.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "537bc3c4a347b87fd52ac6c03a02ab1302962cfd93373c5d7a112cdc337854cc"
+dependencies = [
+ "bitflags 2.9.0",
+ "cfg-if",
+ "cfg_aliases",
+ "libc",
+]
+
 [[package]]
 name = "nom"
 version = "7.1.3"
@@ -2170,6 +2182,7 @@ dependencies = [
  "napi",
  "napi-build",
  "napi-derive",
+ "nix 0.30.0",
  "nom",
  "once_cell",
  "parking_lot",

packages/nx/Cargo.toml

@@ -59,10 +59,11 @@ xxhash-rust = { version = '0.8.5', features = ['xxh3', 'xxh64'] }
 vt100-ctt = { git = "https://github.com/JamesHenry/vt100-rust", rev = "1de895505fe9f697aadac585e4075b8fb45c880d" }
 
 [target.'cfg(windows)'.dependencies]
-winapi = { version = "0.3", features = ["fileapi"] }
+winapi = { version = "0.3", features = ["fileapi", "psapi", "shellapi"] }
 
 [target.'cfg(all(not(windows), not(target_family = "wasm")))'.dependencies]
 mio = "0.8"
+nix = { version = "0.30.0", features = ["process", "signal"] }
 
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
 arboard = "3.4.1"

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

@@ -71,7 +71,7 @@ export class ParallelRunningTasks implements RunningTask {
     }
   }
 
-  async kill(signal?: NodeJS.Signals | number) {
+  async kill(signal?: NodeJS.Signals) {
     await Promise.all(
       this.childProcesses.map(async (p) => {
         try {
@@ -215,7 +215,7 @@ export class SeriallyRunningTasks implements RunningTask {
     throw new Error('Not implemented');
   }
 
-  kill(signal?: NodeJS.Signals | number) {
+  kill(signal?: NodeJS.Signals) {
     return this.currentProcess.kill(signal);
   }
 
@@ -357,7 +357,7 @@ class RunningNodeProcess implements RunningTask {
     this.childProcess.send(message);
   }
 
-  kill(signal?: NodeJS.Signals | number): Promise<void> {
+  kill(signal?: NodeJS.Signals): Promise<void> {
     return new Promise<void>((res, rej) => {
       if (process.platform === 'win32' || process.platform === 'darwin') {
         if (this.childProcess.kill(signal)) {

packages/nx/src/native/index.d.ts

@@ -26,7 +26,7 @@ export declare class AppLifeCycle {
 
 export declare class ChildProcess {
   getParserAndWriter(): ExternalObject<[ParserArc, WriterArc]>
-  kill(): void
+  kill(signal?: NodeJS.Signals): void
   onExit(callback: (message: string) => void): void
   onOutput(callback: (message: string) => void): void
   cleanup(): void

packages/nx/src/native/pseudo_terminal/child_process.rs

@@ -1,3 +1,4 @@
+use super::process_killer::ProcessKiller;
 use crate::native::pseudo_terminal::pseudo_terminal::{ParserArc, WriterArc};
 use crossbeam_channel::Sender;
 use crossbeam_channel::{bounded, Receiver};
@@ -8,7 +9,6 @@ use napi::{
     },
     Env, JsFunction,
 };
-use portable_pty::ChildKiller;
 use std::io::Write;
 use std::sync::{Arc, Mutex, RwLock};
 use tracing::warn;
@@ -21,7 +21,7 @@ pub enum ChildProcessMessage {
 #[napi]
 pub struct ChildProcess {
     parser: Arc<RwLock<Parser>>,
-    process_killer: Box<dyn ChildKiller + Sync + Send>,
+    process_killer: ProcessKiller,
     message_receiver: Receiver<String>,
     pub(crate) wait_receiver: Receiver<String>,
     thread_handles: Vec<Sender<()>>,
@@ -32,7 +32,7 @@ impl ChildProcess {
     pub fn new(
         parser: Arc<RwLock<Parser>>,
         writer_arc: Arc<Mutex<Box<dyn Write + Send>>>,
-        process_killer: Box<dyn ChildKiller + Sync + Send>,
+        process_killer: ProcessKiller,
         message_receiver: Receiver<String>,
         exit_receiver: Receiver<String>,
     ) -> Self {
@@ -51,9 +51,9 @@ impl ChildProcess {
         External::new((self.parser.clone(), self.writer_arc.clone()))
     }
 
-    #[napi]
-    pub fn kill(&mut self) -> anyhow::Result<()> {
-        self.process_killer.kill().map_err(anyhow::Error::from)
+    #[napi(ts_args_type = "signal?: NodeJS.Signals")]
+    pub fn kill(&mut self, signal: Option<&str>) -> anyhow::Result<()> {
+        self.process_killer.kill(signal)
     }
 
     #[napi]

packages/nx/src/native/pseudo_terminal/mod.rs

@@ -10,3 +10,7 @@ pub mod child_process;
 #[cfg_attr(target_os = "macos", path = "mac.rs")]
 #[cfg_attr(not(target_os = "macos"), path = "non_mac.rs")]
 pub mod rust_pseudo_terminal;
+
+#[cfg_attr(windows, path = "process_killer/windows.rs")]
+#[cfg_attr(not(windows), path = "process_killer/unix.rs")]
+mod process_killer;

packages/nx/src/native/pseudo_terminal/process_killer/unix.rs

@@ -0,0 +1,59 @@
+use nix::{
+    sys::signal::{kill, Signal as NixSignal},
+    unistd::Pid,
+};
+
+pub struct ProcessKiller {
+    pid: i32,
+}
+
+impl ProcessKiller {
+    pub fn new(pid: i32) -> Self {
+        Self { pid }
+    }
+
+    pub fn kill(&self, signal: Option<&str>) -> anyhow::Result<()> {
+        let pid = Pid::from_raw(self.pid);
+        match kill(
+            pid,
+            NixSignal::from(
+                Signal::try_from(signal.unwrap_or("SIGINT")).map_err(|e| anyhow::anyhow!(e))?,
+            ),
+        ) {
+            Ok(_) => Ok(()),
+            Err(e) => Err(anyhow::anyhow!("Failed to kill process: {}", e)),
+        }
+    }
+}
+
+enum Signal {
+    SIGTERM,
+    SIGINT,
+    SIGKILL,
+    SIGHUP,
+}
+
+impl TryFrom<&str> for Signal {
+    type Error = String;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        match value {
+            "SIGHUP" => Ok(Signal::SIGHUP),
+            "SIGINT" => Ok(Signal::SIGINT),
+            "SIGKILL" => Ok(Signal::SIGKILL),
+            "SIGTERM" => Ok(Signal::SIGTERM),
+            _ => Err(format!("Invalid signal: {}", value)),
+        }
+    }
+}
+
+impl From<Signal> for NixSignal {
+    fn from(signal: Signal) -> Self {
+        match signal {
+            Signal::SIGTERM => NixSignal::SIGTERM,
+            Signal::SIGINT => NixSignal::SIGINT,
+            Signal::SIGKILL => NixSignal::SIGKILL,
+            Signal::SIGHUP => NixSignal::SIGHUP,
+        }
+    }
+}

packages/nx/src/native/pseudo_terminal/process_killer/windows.rs

@@ -0,0 +1,47 @@
+use std::ptr::null_mut;
+
+use winapi::shared::minwindef::DWORD;
+use winapi::shared::ntdef::HANDLE;
+use winapi::um::processthreadsapi::{OpenProcess, TerminateProcess};
+use winapi::um::winnt::{PROCESS_QUERY_INFORMATION, PROCESS_TERMINATE};
+
+pub struct ProcessKiller {
+    pid: i32,
+}
+
+impl ProcessKiller {
+    pub fn new(pid: i32) -> Self {
+        Self { pid }
+    }
+
+    // windows doesn't have different signals to kill with
+    pub fn kill(&self, _: Option<String>) -> anyhow::Result<()> {
+        let pc = WinProcess::open(self.pid as DWORD).expect("!open");
+        pc.kill()
+            .map_err(|e| anyhow::anyhow!("Failed to kill process {}: {}", self.pid, e))?;
+        Ok(())
+    }
+}
+
+struct WinProcess(HANDLE);
+impl WinProcess {
+    fn open(pid: DWORD) -> anyhow::Result<WinProcess> {
+        // https://msdn.microsoft.com/en-us/library/windows/desktop/ms684320%28v=vs.85%29.aspx
+        let pc = unsafe { OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_TERMINATE, 0, pid) };
+        if pc == null_mut() {
+            anyhow::bail!("Failed to open process with pid {}", pid)
+        } else {
+            Ok(WinProcess(pc))
+        }
+    }
+
+    fn kill(self) -> Result<(), String> {
+        unsafe { TerminateProcess(self.0, 1) };
+        Ok(())
+    }
+}
+impl Drop for WinProcess {
+    fn drop(&mut self) {
+        unsafe { winapi::um::handleapi::CloseHandle(self.0) };
+    }
+}

packages/nx/src/native/pseudo_terminal/pseudo_terminal.rs

@@ -23,7 +23,7 @@ use tracing::log::trace;
 use vt100_ctt::Parser;
 
 use super::os;
-use crate::native::pseudo_terminal::child_process::ChildProcess;
+use crate::native::pseudo_terminal::{child_process::ChildProcess, process_killer::ProcessKiller};
 
 pub struct PseudoTerminal {
     pub pty_pair: PtyPair,
@@ -226,7 +226,11 @@ impl PseudoTerminal {
             trace!("Enabling raw mode");
             enable_raw_mode().expect("Failed to enter raw terminal mode");
         }
-        let process_killer = child.clone_killer();
+        let process_killer = ProcessKiller::new(
+            child
+                .process_id()
+                .expect("unable to determine child process id") as i32,
+        );
 
         trace!("Getting running clone");
         let running_clone = self.running.clone();
@@ -318,7 +322,7 @@ mod tests {
         while i < 10 {
             println!("Running {}", i);
             let cp1 = pseudo_terminal
-                .run_command(String::from("whoami"), None, None, None)
+                .run_command(String::from("whoami"), None, None, None, None, None)
                 .unwrap();
             cp1.wait_receiver.recv().unwrap();
             i += 1;

packages/nx/src/tasks-runner/life-cycle.ts

@@ -27,7 +27,7 @@ interface RustRunningTask extends RunningTask {
 
   onExit(cb: (code: number, terminalOutput: string) => void): void;
 
-  kill(signal?: NodeJS.Signals | number): Promise<void> | void;
+  kill(signal?: NodeJS.Signals): Promise<void> | void;
 }
 
 export interface LifeCycle {

packages/nx/src/tasks-runner/pseudo-terminal.ts

@@ -6,15 +6,15 @@ import { PseudoIPCServer } from './pseudo-ipc';
 import { RunningTask } from './running-tasks/running-task';
 
 // Register single event listeners for all pseudo-terminal instances
-const pseudoTerminalShutdownCallbacks: Array<() => void> = [];
+const pseudoTerminalShutdownCallbacks: Array<(s?: NodeJS.Signals) => void> = [];
 process.on('SIGINT', () => {
-  pseudoTerminalShutdownCallbacks.forEach((cb) => cb());
+  pseudoTerminalShutdownCallbacks.forEach((cb) => cb('SIGINT'));
 });
 process.on('SIGTERM', () => {
-  pseudoTerminalShutdownCallbacks.forEach((cb) => cb());
+  pseudoTerminalShutdownCallbacks.forEach((cb) => cb('SIGTERM'));
 });
 process.on('SIGHUP', () => {
-  pseudoTerminalShutdownCallbacks.forEach((cb) => cb());
+  pseudoTerminalShutdownCallbacks.forEach((cb) => cb('SIGHUP'));
 });
 process.on('exit', () => {
   pseudoTerminalShutdownCallbacks.forEach((cb) => cb());
@@ -56,10 +56,10 @@ export class PseudoTerminal {
     this.initialized = true;
   }
 
-  shutdown() {
+  shutdown(s?: NodeJS.Signals) {
     for (const cp of this.childProcesses) {
       try {
-        cp.kill();
+        cp.kill(s);
       } catch {}
     }
     if (this.initialized) {
@@ -192,10 +192,10 @@ export class PseudoTtyProcess implements RunningTask {
     this.outputCallbacks.push(callback);
   }
 
-  kill(): void {
+  kill(s?: NodeJS.Signals): void {
     if (this.isAlive) {
       try {
-        this.childProcess.kill();
+        this.childProcess.kill(s);
       } catch {
         // when the child process completes before we explicitly call kill, this will throw
         // do nothing

packages/nx/src/tasks-runner/running-tasks/batch-process.ts

@@ -76,7 +76,7 @@ export class BatchProcess {
     }
   }
 
-  kill(signal?: NodeJS.Signals | number): void {
+  kill(signal?: NodeJS.Signals): void {
     if (this.childProcess.connected) {
       this.childProcess.kill(signal);
     }

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

@@ -86,7 +86,7 @@ export class NodeChildProcessWithNonDirectOutput implements RunningTask {
     }
   }
 
-  public kill(signal?: NodeJS.Signals | number) {
+  public kill(signal?: NodeJS.Signals) {
     if (this.childProcess.connected) {
       this.childProcess.kill(signal);
     }
@@ -209,7 +209,7 @@ export class NodeChildProcessWithDirectOutput implements RunningTask {
     return this.terminalOutput;
   }
 
-  kill(signal?: NodeJS.Signals | number): void {
+  kill(signal?: NodeJS.Signals): void {
     if (this.childProcess.connected) {
       this.childProcess.kill(signal);
     }

packages/nx/src/tasks-runner/running-tasks/running-task.ts

@@ -5,7 +5,7 @@ export abstract class RunningTask {
 
   abstract onExit(cb: (code: number) => void): void;
 
-  abstract kill(signal?: NodeJS.Signals | number): Promise<void> | void;
+  abstract kill(signal?: NodeJS.Signals): Promise<void> | void;
 
   abstract onOutput?(cb: (output: string) => void): void;