Kill subprocesses correctly on each platform on Ctrl-C (#13)

* Kill subprocesses correctly on each platform on Ctrl-C

* Fix Unix build errors

* Fix more build errors

* Fix warnings on Unix

* Enable I/O for Unix process runtime

* Enter runtime before calling Command::spawn

* Fix clippy error

Cargo.toml

@@ -12,7 +12,6 @@ readme = "README.md"
 [dependencies]
 anyhow = "1.0.43"
 atty = "0.2.14"
-command-group = "1.0.8"
 dialoguer = "0.9.0"
 dirs = "3.0.2"
 env_logger = "0.9.0"
@@ -28,9 +27,14 @@ toml = "0.5.8"
 toml_edit = "0.14.4"
 zip = "0.5.13"
 
-[target.'cfg(target_os = "windows")'.dependencies]
+[target.'cfg(windows)'.dependencies]
+command-group = "1.0.8"
 winreg = "0.10.1"
 
+[target.'cfg(unix)'.dependencies]
+tokio = { version = "1.18.2", features = ["macros", "sync", "process"] }
+signal-hook = "0.3.14"
+
 [dev-dependencies]
 tempfile = "3.3.0"
 serde_json = "1.0.66"

src/main.rs

@@ -3,6 +3,7 @@ mod config;
 mod home;
 mod ident;
 mod manifest;
+mod process;
 mod system_path;
 mod tool_alias;
 mod tool_id;

src/process/mod.rs

@@ -0,0 +1,11 @@
+#[cfg(windows)]
+mod windows;
+
+#[cfg(windows)]
+pub use windows::run;
+
+#[cfg(unix)]
+mod unix;
+
+#[cfg(unix)]
+pub use unix::run;

src/process/unix.rs

@@ -0,0 +1,67 @@
+//! On Unix, we use tokio to spawn processes so that we can listen for signals
+//! and wait for process completion at the same time.
+
+use std::path::Path;
+use std::thread;
+
+use anyhow::Context;
+use signal_hook::consts::signal::{SIGABRT, SIGINT, SIGQUIT, SIGTERM};
+use signal_hook::iterator::Signals;
+use tokio::process::Command;
+use tokio::sync::oneshot;
+
+pub fn run(exe_path: &Path, args: Vec<String>) -> anyhow::Result<i32> {
+    let (kill_tx, kill_rx) = oneshot::channel();
+
+    // Spawn a thread dedicated to listening for signals and relaying them to
+    // our async runtime.
+    let (signal_thread, signal_handle) = {
+        let mut signals = Signals::new(&[SIGABRT, SIGINT, SIGQUIT, SIGTERM]).unwrap();
+        let signal_handle = signals.handle();
+
+        let thread = thread::spawn(move || {
+            if let Some(signal) = signals.into_iter().next() {
+                kill_tx.send(signal).ok();
+            }
+        });
+
+        (thread, signal_handle)
+    };
+
+    let runtime = tokio::runtime::Builder::new_current_thread()
+        .enable_io()
+        .build()
+        .context("could not create tokio runtime")?;
+
+    let _guard = runtime.enter();
+
+    let mut child = Command::new(exe_path)
+        .args(args)
+        .spawn()
+        .with_context(|| format!("could not spawn {}", exe_path.display()))?;
+
+    let code = runtime.block_on(async move {
+        tokio::select! {
+            // If the child exits cleanly, we can return its exit code directly.
+            // I wish everything were this tidy.
+            status = child.wait() => {
+                let code = status.ok().and_then(|s| s.code()).unwrap_or(1);
+                signal_handle.close();
+                signal_thread.join().unwrap();
+
+                code
+            }
+
+            // If we received a signal while the process was running, murder it
+            // and exit immediately with the correct error code.
+            code = kill_rx => {
+                child.kill().await.ok();
+                signal_handle.close();
+                signal_thread.join().unwrap();
+                std::process::exit(128 + code.unwrap_or(0));
+            }
+        }
+    });
+
+    Ok(code)
+}

src/process/windows.rs

@@ -0,0 +1,20 @@
+//! On Windows, we use command_group to spawn processes in a job group that will
+//! be automatically cleaned up when this process exits.
+
+use std::path::Path;
+use std::process::Command;
+
+use anyhow::Context;
+use command_group::CommandGroup;
+
+pub fn run(exe_path: &Path, args: Vec<String>) -> anyhow::Result<i32> {
+    // On Windows, using a job group here will cause the subprocess to terminate
+    // automatically when Aftman is terminated.
+    let mut child = Command::new(exe_path)
+        .args(args)
+        .group_spawn()
+        .with_context(|| format!("Could not spawn {}", exe_path.display()))?;
+
+    let status = child.wait()?;
+    Ok(status.code().unwrap_or(1))
+}

src/system_path/mod.rs

@@ -5,7 +5,7 @@ mod windows;
 pub use windows::add;
 
 #[cfg(not(target_os = "windows"))]
-pub fn add(path: &std::path::Path) -> anyhow::Result<bool> {
+pub fn add(_path: &std::path::Path) -> anyhow::Result<bool> {
     log::debug!("Not adding value to path because this platform is not supported.");
     Ok(false)
 }

src/tool_storage.rs

@@ -6,10 +6,8 @@ use std::fmt::Write;
 use std::io::{self, BufWriter, Read};
 use std::io::{Seek, Write as _};
 use std::path::{Path, PathBuf};
-use std::process::Command;
 
 use anyhow::{bail, Context};
-use command_group::CommandGroup;
 use fs_err::File;
 use once_cell::unsync::OnceCell;
 
@@ -74,9 +72,10 @@ impl ToolStorage {
         self.install_exact(id, TrustMode::Check)?;
 
         let exe_path = self.exe_path(id);
-        let status = Command::new(exe_path).args(args).group_status().unwrap();
-
-        Ok(status.code().unwrap_or(1))
+        let code = crate::process::run(&exe_path, args).with_context(|| {
+            format!("Failed to run tool {id}, your installation may be corrupt.")
+        })?;
+        Ok(code)
     }
 
     pub fn update_links(&self) -> anyhow::Result<()> {