internal: Funnel all child process spawning through stdx::process

clippy.toml

@@ -6,4 +6,7 @@ disallowed-types = [
 
 disallowed-methods = [
     { path = "std::process::Command::new", reason = "use `toolchain::command` instead as it forces the choice of a working directory" },
+    { path = "std::process::Command::spawn", reason = "use `stdx::process::JodChild::spawn` instead to prevent processes leaking on exit" },
+    { path = "std::process::Command::output", reason = "use `stdx::process::output` instead to prevent processes leaking on exit" },
+    { path = "std::process::Command::status", reason = "use `stdx::process` instead to prevent processes leaking on exit" },
 ]

crates/ide/src/expand_macro.rs

@@ -259,14 +259,12 @@ fn _format(
     cmd.arg("--edition");
     cmd.arg(edition.to_string());
 
-    let mut rustfmt = cmd
-        .stdin(std::process::Stdio::piped())
+    cmd.stdin(std::process::Stdio::piped())
         .stdout(std::process::Stdio::piped())
-        .stderr(std::process::Stdio::piped())
-        .spawn()
-        .ok()?;
+        .stderr(std::process::Stdio::piped());
+    let mut rustfmt = stdx::process::JodChild::spawn(&mut cmd).ok()?;
 
-    std::io::Write::write_all(&mut rustfmt.stdin.as_mut()?, expansion.as_bytes()).ok()?;
+    std::io::Write::write_all(&mut rustfmt.stdin().as_mut()?, expansion.as_bytes()).ok()?;
 
     let output = rustfmt.wait_with_output().ok()?;
     let captured_stdout = String::from_utf8(output.stdout).ok()?;

crates/ide/src/syntax_highlighting/tests.rs

@@ -447,7 +447,7 @@ macro_rules! void_2024 {
 }
 
 "#,
-        expect_file![format!("./test_data/highlight_keywords_macros.html")],
+        expect_file!["./test_data/highlight_keywords_macros.html"],
         false,
     );
 }

crates/proc-macro-api/src/process.rs

@@ -4,7 +4,7 @@ use std::{
     fmt::Debug,
     io::{self, BufRead, BufReader, Read, Write},
     panic::AssertUnwindSafe,
-    process::{Child, ChildStdin, ChildStdout, Command, Stdio},
+    process::{ChildStdin, ChildStdout, Command, Stdio},
     sync::{
         Arc, Mutex, OnceLock,
         atomic::{AtomicU32, Ordering},
@@ -14,7 +14,7 @@ use std::{
 use paths::AbsPath;
 use semver::Version;
 use span::Span;
-use stdx::JodChild;
+use stdx::process::JodChild;
 
 use crate::{
     ProcMacro, ProcMacroKind, ProtocolFormat, ServerError,
@@ -66,7 +66,7 @@ impl ProcessExit for Process {
             Ok(Some(status)) => {
                 let mut msg = String::new();
                 if !status.success()
-                    && let Some(stderr) = self.child.stderr.as_mut()
+                    && let Some(stderr) = self.child.stderr().as_mut()
                 {
                     _ = stderr.read_to_string(&mut msg);
                 }
@@ -112,9 +112,9 @@ impl ProcMacroServerProcess {
             version,
             || {
                 #[expect(clippy::disallowed_methods)]
-                Command::new(process_path)
-                    .arg("--version")
-                    .output()
+                let mut cmd = Command::new(process_path);
+                cmd.arg("--version");
+                stdx::process::output(&mut cmd)
                     .map(|output| String::from_utf8_lossy(&output.stdout).trim().to_owned())
                     .unwrap_or_else(|_| "unknown version".to_owned())
             },
@@ -390,14 +390,14 @@ impl Process {
         >,
         format: Option<&str>,
     ) -> io::Result<Process> {
-        let child = JodChild(mk_child(path, env, format)?);
+        let child = mk_child(path, env, format)?;
         Ok(Process { child })
     }
 
     /// Retrieves stdin and stdout handles for the process.
     fn stdio(&mut self) -> Option<(ChildStdin, BufReader<ChildStdout>)> {
-        let stdin = self.child.stdin.take()?;
-        let stdout = self.child.stdout.take()?;
+        let stdin = self.child.stdin().take()?;
+        let stdout = self.child.stdout().take()?;
         let read = BufReader::new(stdout);
 
         Some((stdin, read))
@@ -411,7 +411,7 @@ fn mk_child<'a>(
         Item = (impl AsRef<std::ffi::OsStr>, &'a Option<impl 'a + AsRef<std::ffi::OsStr>>),
     >,
     format: Option<&str>,
-) -> io::Result<Child> {
+) -> io::Result<JodChild> {
     #[allow(clippy::disallowed_methods)]
     let mut cmd = Command::new(path);
     for env in extra_env {
@@ -435,5 +435,5 @@ fn mk_child<'a>(
         path_var.push(std::env::var_os("PATH").unwrap_or_default());
         cmd.env("PATH", path_var);
     }
-    cmd.spawn()
+    JodChild::spawn(&mut cmd)
 }

crates/project-model/src/cargo_workspace.rs

@@ -1,6 +1,6 @@
 //! See [`CargoWorkspace`].
 
-use std::{borrow::Cow, ops, str::from_utf8};
+use std::{borrow::Cow, ops, process::Command, str::from_utf8};
 
 use anyhow::Context;
 use base_db::Env;
@@ -721,12 +721,12 @@ impl FetchMetadata {
         let no_deps_result = if no_deps {
             command.no_deps();
             cargo_command = command.cargo_command();
-            command.exec()
+            exec_cargo_metadata(command.cargo_command())
         } else {
             let mut no_deps_command = command.clone();
             no_deps_command.no_deps();
             cargo_command = no_deps_command.cargo_command();
-            no_deps_command.exec()
+            exec_cargo_metadata(no_deps_command.cargo_command())
         }
         .with_context(|| format!("Failed to run `{cargo_command:?}`"));
 
@@ -836,3 +836,19 @@ impl FetchMetadata {
         res
     }
 }
+
+fn exec_cargo_metadata(mut cmd: Command) -> anyhow::Result<cargo_metadata::Metadata> {
+    let output = stdx::process::output(&mut cmd)?;
+    if !output.status.success() {
+        return Err(cargo_metadata::Error::CargoMetadata {
+            stderr: String::from_utf8(output.stderr)
+                .unwrap_or_else(|_| String::from("Couldn't read `stderr` output")),
+        }
+        .into());
+    }
+    let stdout = from_utf8(&output.stdout)?
+        .lines()
+        .find(|line| line.starts_with('{'))
+        .ok_or(cargo_metadata::Error::NoJson)?;
+    Ok(MetadataCommand::parse(stdout)?)
+}

crates/project-model/src/lib.rs

@@ -211,7 +211,7 @@ impl fmt::Display for ProjectManifest {
 }
 
 fn utf8_stdout(cmd: &mut Command) -> anyhow::Result<String> {
-    let output = cmd.output().with_context(|| format!("{cmd:?} failed"))?;
+    let output = stdx::process::output(cmd).with_context(|| format!("{cmd:?} failed"))?;
     if !output.status.success() {
         match String::from_utf8(output.stderr) {
             Ok(stderr) if !stderr.is_empty() => {

crates/project-model/src/sysroot.rs

@@ -194,7 +194,9 @@ impl Sysroot {
                 cmd.arg(tool.name());
                 (|| {
                     Some(Utf8PathBuf::from(
-                        String::from_utf8(cmd.output().ok()?.stdout).ok()?.trim_end(),
+                        String::from_utf8(stdx::process::output(&mut cmd).ok()?.stdout)
+                            .ok()?
+                            .trim_end(),
                     ))
                 })()
                 .unwrap_or_else(|| Utf8PathBuf::from(tool.name()))

crates/rust-analyzer/Cargo.toml

@@ -52,7 +52,6 @@ walkdir = "2.5.0"
 semver.workspace = true
 memchr = "2.7.5"
 cargo_metadata.workspace = true
-process-wrap.workspace = true
 dhat = { version = "0.3.3", optional = true }
 
 cfg.workspace = true

crates/rust-analyzer/src/bin/main.rs

@@ -51,6 +51,10 @@ fn actual_main() -> anyhow::Result<ExitCode> {
         eprintln!("Failed to setup logging: {e:#}");
     }
 
+    // After logging so that failures to set this up are visible, but before
+    // anything that may spawn child processes.
+    stdx::process::kill_descendants_on_exit();
+
     let verbosity = flags.verbosity();
 
     match flags.subcommand {

crates/rust-analyzer/src/bin/rustc_wrapper.rs

@@ -47,11 +47,8 @@ fn run_rustc_skipping_cargo_checking(
 
 fn run_rustc(rustc_executable: OsString, args: Vec<OsString>) -> io::Result<ExitCode> {
     #[allow(clippy::disallowed_methods)]
-    let mut child = Command::new(rustc_executable)
-        .args(args)
-        .stdin(Stdio::inherit())
-        .stdout(Stdio::inherit())
-        .stderr(Stdio::inherit())
-        .spawn()?;
+    let mut cmd = Command::new(rustc_executable);
+    cmd.args(args).stdin(Stdio::inherit()).stdout(Stdio::inherit()).stderr(Stdio::inherit());
+    let mut child = stdx::process::JodChild::spawn(&mut cmd)?;
     Ok(ExitCode::from(child.wait()?.code().unwrap_or(102) as u8))
 }

crates/rust-analyzer/src/command.rs

@@ -13,8 +13,7 @@ use std::{
 use anyhow::Context;
 use crossbeam_channel::Sender;
 use paths::Utf8PathBuf;
-use process_wrap::std::{ChildWrapper, CommandWrap};
-use stdx::process::streaming_output;
+use stdx::process::{JodChild, streaming_output};
 
 /// This trait abstracts parsing one line of JSON output into a Rust
 /// data type.
@@ -117,23 +116,11 @@ impl<T: Sized + Send + 'static> CommandActor<T> {
     }
 }
 
-/// 'Join On Drop' wrapper for a child process.
-///
-/// This wrapper kills the process when the wrapper is dropped.
-struct JodGroupChild(Box<dyn ChildWrapper>);
-
-impl Drop for JodGroupChild {
-    fn drop(&mut self) {
-        _ = self.0.kill();
-        _ = self.0.wait();
-    }
-}
-
 /// A handle to a shell command, such as cargo for diagnostics (flycheck).
 pub(crate) struct CommandHandle<T> {
     /// The handle to the actual child process. As we cannot cancel directly from with
     /// a read syscall dropping and therefore terminating the process is our best option.
-    child: JodGroupChild,
+    child: JodChild,
     thread: stdx::thread::JoinHandle<io::Result<(bool, String)>>,
     program: OsString,
     arguments: Vec<OsString>,
@@ -164,18 +151,11 @@ impl<T: Sized + Send + 'static> CommandHandle<T> {
         let arguments = command.get_args().map(|arg| arg.into()).collect::<Vec<OsString>>();
         let current_dir = command.get_current_dir().map(|arg| arg.to_path_buf());
 
-        let mut child = CommandWrap::from(command);
-        #[cfg(unix)]
-        child.wrap(process_wrap::std::ProcessSession);
-        #[cfg(windows)]
-        child.wrap(process_wrap::std::JobObject);
-        let mut child = child
-            .spawn()
-            .map(JodGroupChild)
-            .with_context(|| "Failed to spawn command: {child:?}")?;
+        let mut child = JodChild::spawn_grouped(command)
+            .with_context(|| format!("Failed to spawn command: {program:?}"))?;
 
-        let stdout = child.0.stdout().take().unwrap();
-        let stderr = child.0.stderr().take().unwrap();
+        let stdout = child.stdout().take().unwrap();
+        let stderr = child.stderr().take().unwrap();
 
         let actor = CommandActor::<T>::new(parser, sender, stdout, stderr);
         let thread =
@@ -186,12 +166,12 @@ impl<T: Sized + Send + 'static> CommandHandle<T> {
     }
 
     pub(crate) fn cancel(mut self) {
-        let _ = self.child.0.kill();
-        let _ = self.child.0.wait();
+        let _ = self.child.kill();
+        let _ = self.child.wait();
     }
 
     pub(crate) fn join(mut self) -> io::Result<()> {
-        let exit_status = self.child.0.wait()?;
+        let exit_status = self.child.wait()?;
         let (read_at_least_one_message, error) = self.thread.join()?;
         if read_at_least_one_message || exit_status.success() {
             Ok(())
@@ -203,7 +183,7 @@ impl<T: Sized + Send + 'static> CommandHandle<T> {
     }
 
     pub(crate) fn has_exited(&mut self) -> bool {
-        match self.child.0.try_wait() {
+        match self.child.try_wait() {
             Ok(Some(_exit_code)) => {
                 // We have an exit code.
                 true

crates/rust-analyzer/src/config.rs

@@ -4273,7 +4273,10 @@ mod tests {
         }
 
         let package_json_path = project_root().join("editors/code/package.json");
-        let mut package_json = fs::read_to_string(&package_json_path).unwrap();
+        // Normalize line endings for the marker search below, in case the working
+        // tree was checked out with CRLF line endings.
+        let mut package_json =
+            fs::read_to_string(&package_json_path).unwrap().replace("\r\n", "\n");
 
         let start_marker =
             "            {\n                \"title\": \"$generated-start\"\n            },\n";

crates/rust-analyzer/src/handlers/request.rs

@@ -2526,14 +2526,11 @@ fn run_rustfmt(
     let output = {
         let _p = tracing::info_span!("rustfmt", ?command).entered();
 
-        let mut rustfmt = command
-            .stdin(Stdio::piped())
-            .stdout(Stdio::piped())
-            .stderr(Stdio::piped())
-            .spawn()
+        command.stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped());
+        let mut rustfmt = stdx::process::JodChild::spawn(&mut command)
             .context(format!("Failed to spawn {command:?}"))?;
 
-        rustfmt.stdin.as_mut().unwrap().write_all(file.as_bytes())?;
+        rustfmt.stdin().as_mut().unwrap().write_all(file.as_bytes())?;
 
         rustfmt.wait_with_output()?
     };

crates/rust-analyzer/tests/slow-tests/main.rs

@@ -669,10 +669,10 @@ fn test_format_document_range() {
 
     // This test requires a nightly toolchain, so skip if it's not available.
     let cwd = std::env::current_dir().unwrap_or_default();
-    let has_nightly_rustfmt = toolchain::command("rustfmt", cwd, &FxHashMap::default())
-        .args(["+nightly", "--version"])
-        .output()
-        .is_ok_and(|out| out.status.success());
+    let mut rustfmt = toolchain::command("rustfmt", cwd, &FxHashMap::default());
+    rustfmt.args(["+nightly", "--version"]);
+    let has_nightly_rustfmt =
+        stdx::process::output(&mut rustfmt).is_ok_and(|out| out.status.success());
     if !has_nightly_rustfmt {
         tracing::warn!("skipping test_format_document_range: nightly rustfmt not available");
         return;

crates/stdx/Cargo.toml

@@ -24,9 +24,17 @@ crossbeam-utils = "0.8.21"
 [target.'cfg(unix)'.dependencies]
 libc.workspace = true
 
+[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
+process-wrap.workspace = true
+
 [target.'cfg(windows)'.dependencies]
 miow = "0.6.0"
-windows-sys = { version = "0.61", features = ["Win32_Foundation"] }
+windows-sys = { version = "0.61", features = [
+    "Win32_Foundation",
+    "Win32_Security",
+    "Win32_System_JobObjects",
+    "Win32_System_Threading",
+] }
 
 [features]
 # Uncomment to enable for the whole crate graph