Forbid std::process::Command spawning, replace with smol where appropriate (#38894)

std commands can block for an arbitrary duration and so runs risk of
blocking tasks for too long. This replaces all such uses where sensible
with async processes.

Release Notes:

- N/A *or* Added/Fixed/Improved ...

Author: Veykril

Cargo.toml

@@ -862,6 +862,7 @@ todo = "deny"
 declare_interior_mutable_const = "deny"
 
 redundant_clone = "deny"
+disallowed_methods = "deny"
 
 # We currently do not restrict any style rules
 # as it slows down shipping code to Zed.

clippy.toml

@@ -5,3 +5,14 @@ ignore-interior-mutability = [
     # and Hash impls do not use fields with interior mutability.
     "agent::context::AgentContextKey"
 ]
+disallowed-methods = [
+    { path = "std::process::Command::spawn", reason = "Spawning `std::process::Command` can block the current thread for an unknown duration", replacement = "smol::process::Command::spawn" },
+    { path = "std::process::Command::output", reason = "Spawning `std::process::Command` can block the current thread for an unknown duration", replacement = "smol::process::Command::output" },
+    { path = "std::process::Command::status", reason = "Spawning `std::process::Command` can block the current thread for an unknown duration", replacement = "smol::process::Command::status" },
+]
+disallowed-types = [
+    # { path = "std::collections::HashMap", replacement = "collections::HashMap" },
+    # { path = "std::collections::HashSet", replacement = "collections::HashSet" },
+    # { path = "indexmap::IndexSet", replacement = "collections::IndexSet" },
+    # { path = "indexmap::IndexMap", replacement = "collections::IndexMap" },
+]

crates/auto_update/src/auto_update.rs

@@ -310,10 +310,10 @@ impl AutoUpdater {
         // the app after an update, we use `set_restart_path` to run the auto
         // update helper instead of the app, so that it can overwrite the app
         // and then spawn the new binary.
-        let quit_subscription = Some(cx.on_app_quit(|_, _| async move {
-            #[cfg(target_os = "windows")]
-            finalize_auto_update_on_quit();
-        }));
+        #[cfg(target_os = "windows")]
+        let quit_subscription = Some(cx.on_app_quit(|_, _| finalize_auto_update_on_quit()));
+        #[cfg(not(target_os = "windows"))]
+        let quit_subscription = None;
 
         cx.on_app_restart(|this, _| {
             this.quit_subscription.take();
@@ -942,11 +942,12 @@ async fn install_release_windows(downloaded_installer: PathBuf) -> Result<Option
     let helper_path = std::env::current_exe()?
         .parent()
         .context("No parent dir for Zed.exe")?
-        .join("tools\\auto_update_helper.exe");
+        .join("tools")
+        .join("auto_update_helper.exe");
     Ok(Some(helper_path))
 }
 
-pub fn finalize_auto_update_on_quit() {
+pub async fn finalize_auto_update_on_quit() {
     let Some(installer_path) = std::env::current_exe()
         .ok()
         .and_then(|p| p.parent().map(|p| p.join("updates")))
@@ -959,12 +960,14 @@ pub fn finalize_auto_update_on_quit() {
     if flag_file.exists()
         && let Some(helper) = installer_path
             .parent()
-            .map(|p| p.join("tools\\auto_update_helper.exe"))
+            .map(|p| p.join("tools").join("auto_update_helper.exe"))
     {
-        let mut command = std::process::Command::new(helper);
+        let mut command = smol::process::Command::new(helper);
         command.arg("--launch");
         command.arg("false");
-        let _ = command.spawn();
+        if let Ok(mut cmd) = command.spawn() {
+            _ = cmd.status().await;
+        }
     }
 }
 

crates/auto_update_helper/src/updater.rs

@@ -160,6 +160,7 @@ pub(crate) fn perform_update(app_dir: &Path, hwnd: Option<isize>, launch: bool)
         }
     }
     if launch {
+        #[allow(clippy::disallowed_methods, reason = "doesn't run in the main binary")]
         let _ = std::process::Command::new(app_dir.join("Zed.exe"))
             .creation_flags(CREATE_NEW_PROCESS_GROUP.0)
             .spawn();

crates/cli/build.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::disallowed_methods, reason = "build scripts are exempt")]
 use std::process::Command;
 
 fn main() {

crates/cli/src/main.rs

@@ -1,3 +1,7 @@
+#![allow(
+    clippy::disallowed_methods,
+    reason = "We are not in an async environment, so std::process::Command is fine"
+)]
 #![cfg_attr(
     any(target_os = "linux", target_os = "freebsd", target_os = "windows"),
     allow(dead_code)

crates/collections/src/collections.rs

@@ -1,27 +1,8 @@
-#[cfg(feature = "test-support")]
 pub type HashMap<K, V> = FxHashMap<K, V>;
-
-#[cfg(feature = "test-support")]
 pub type HashSet<T> = FxHashSet<T>;
-
-#[cfg(feature = "test-support")]
 pub type IndexMap<K, V> = indexmap::IndexMap<K, V, rustc_hash::FxBuildHasher>;
-
-#[cfg(feature = "test-support")]
 pub type IndexSet<T> = indexmap::IndexSet<T, rustc_hash::FxBuildHasher>;
 
-#[cfg(not(feature = "test-support"))]
-pub type HashMap<K, V> = std::collections::HashMap<K, V>;
-
-#[cfg(not(feature = "test-support"))]
-pub type HashSet<T> = std::collections::HashSet<T>;
-
-#[cfg(not(feature = "test-support"))]
-pub type IndexMap<K, V> = indexmap::IndexMap<K, V>;
-
-#[cfg(not(feature = "test-support"))]
-pub type IndexSet<T> = indexmap::IndexSet<T>;
-
 pub use indexmap::Equivalent;
 pub use rustc_hash::FxHasher;
 pub use rustc_hash::{FxHashMap, FxHashSet};

crates/crashes/src/crashes.rs

@@ -3,6 +3,7 @@ use log::info;
 use minidumper::{Client, LoopAction, MinidumpBinary};
 use release_channel::{RELEASE_CHANNEL, ReleaseChannel};
 use serde::{Deserialize, Serialize};
+use smol::process::Command;
 
 #[cfg(target_os = "macos")]
 use std::sync::atomic::AtomicU32;
@@ -12,7 +13,7 @@ use std::{
     io,
     panic::{self, PanicHookInfo},
     path::{Path, PathBuf},
-    process::{self, Command},
+    process::{self},
     sync::{
         Arc, OnceLock,
         atomic::{AtomicBool, Ordering},
@@ -53,13 +54,13 @@ pub async fn init(crash_init: InitCrashHandler) {
     // used by the crash handler isn't destroyed correctly which causes it to stay on the file
     // system and block further attempts to initialize crash handlers with that socket path.
     let socket_name = paths::temp_dir().join(format!("zed-crash-handler-{zed_pid}"));
-    #[allow(unused)]
-    let server_pid = Command::new(exe)
+    let _crash_handler = Command::new(exe)
         .arg("--crash-handler")
         .arg(&socket_name)
         .spawn()
-        .expect("unable to spawn server process")
-        .id();
+        .expect("unable to spawn server process");
+    #[cfg(target_os = "linux")]
+    let server_pid = _crash_handler.id();
     info!("spawning crash handler process");
 
     let mut elapsed = Duration::ZERO;

crates/explorer_command_injector/src/explorer_command_injector.rs

@@ -77,6 +77,7 @@ impl IExplorerCommand_Impl for ExplorerCommandInjector_Impl {
         for idx in 0..count {
             let item = unsafe { items.GetItemAt(idx)? };
             let item_path = unsafe { item.GetDisplayName(SIGDN_FILESYSPATH)?.to_string()? };
+            #[allow(clippy::disallowed_methods, reason = "no async context in sight..")]
             std::process::Command::new(&zed_exe)
                 .arg(&item_path)
                 .spawn()

crates/extension/src/extension_builder.rs

@@ -142,7 +142,7 @@ impl ExtensionBuilder {
         manifest: &mut ExtensionManifest,
         options: CompileExtensionOptions,
     ) -> anyhow::Result<()> {
-        self.install_rust_wasm_target_if_needed()?;
+        self.install_rust_wasm_target_if_needed().await?;
 
         let cargo_toml_content = fs::read_to_string(extension_dir.join("Cargo.toml"))?;
         let cargo_toml: CargoToml = toml::from_str(&cargo_toml_content)?;
@@ -151,7 +151,7 @@ impl ExtensionBuilder {
             "compiling Rust crate for extension {}",
             extension_dir.display()
         );
-        let output = util::command::new_std_command("cargo")
+        let output = util::command::new_smol_command("cargo")
             .args(["build", "--target", RUST_TARGET])
             .args(options.release.then_some("--release"))
             .arg("--target-dir")
@@ -160,6 +160,7 @@ impl ExtensionBuilder {
             .env("RUSTC_WRAPPER", "")
             .current_dir(extension_dir)
             .output()
+            .await
             .context("failed to run `cargo`")?;
         if !output.status.success() {
             bail!(
@@ -235,7 +236,8 @@ impl ExtensionBuilder {
             &grammar_repo_dir,
             &grammar_metadata.repository,
             &grammar_metadata.rev,
-        )?;
+        )
+        .await?;
 
         let base_grammar_path = grammar_metadata
             .path
@@ -248,7 +250,7 @@ impl ExtensionBuilder {
         let scanner_path = src_path.join("scanner.c");
 
         log::info!("compiling {grammar_name} parser");
-        let clang_output = util::command::new_std_command(&clang_path)
+        let clang_output = util::command::new_smol_command(&clang_path)
             .args(["-fPIC", "-shared", "-Os"])
             .arg(format!("-Wl,--export=tree_sitter_{grammar_name}"))
             .arg("-o")
@@ -258,6 +260,7 @@ impl ExtensionBuilder {
             .arg(&parser_path)
             .args(scanner_path.exists().then_some(scanner_path))
             .output()
+            .await
             .context("failed to run clang")?;
 
         if !clang_output.status.success() {
@@ -271,15 +274,16 @@ impl ExtensionBuilder {
         Ok(())
     }
 
-    fn checkout_repo(&self, directory: &Path, url: &str, rev: &str) -> Result<()> {
+    async fn checkout_repo(&self, directory: &Path, url: &str, rev: &str) -> Result<()> {
         let git_dir = directory.join(".git");
 
         if directory.exists() {
-            let remotes_output = util::command::new_std_command("git")
+            let remotes_output = util::command::new_smol_command("git")
                 .arg("--git-dir")
                 .arg(&git_dir)
                 .args(["remote", "-v"])
-                .output()?;
+                .output()
+                .await?;
             let has_remote = remotes_output.status.success()
                 && String::from_utf8_lossy(&remotes_output.stdout)
                     .lines()
@@ -298,22 +302,24 @@ impl ExtensionBuilder {
             fs::create_dir_all(directory).with_context(|| {
                 format!("failed to create grammar directory {}", directory.display(),)
             })?;
-            let init_output = util::command::new_std_command("git")
+            let init_output = util::command::new_smol_command("git")
                 .arg("init")
                 .current_dir(directory)
-                .output()?;
+                .output()
+                .await?;
             if !init_output.status.success() {
                 bail!(
                     "failed to run `git init` in directory '{}'",
                     directory.display()
                 );
             }
 
-            let remote_add_output = util::command::new_std_command("git")
+            let remote_add_output = util::command::new_smol_command("git")
                 .arg("--git-dir")
                 .arg(&git_dir)
                 .args(["remote", "add", "origin", url])
                 .output()
+                .await
                 .context("failed to execute `git remote add`")?;
             if !remote_add_output.status.success() {
                 bail!(
@@ -323,19 +329,21 @@ impl ExtensionBuilder {
             }
         }
 
-        let fetch_output = util::command::new_std_command("git")
+        let fetch_output = util::command::new_smol_command("git")
             .arg("--git-dir")
             .arg(&git_dir)
             .args(["fetch", "--depth", "1", "origin", rev])
             .output()
+            .await
             .context("failed to execute `git fetch`")?;
 
-        let checkout_output = util::command::new_std_command("git")
+        let checkout_output = util::command::new_smol_command("git")
             .arg("--git-dir")
             .arg(&git_dir)
             .args(["checkout", rev])
             .current_dir(directory)
             .output()
+            .await
             .context("failed to execute `git checkout`")?;
         if !checkout_output.status.success() {
             if !fetch_output.status.success() {
@@ -356,11 +364,12 @@ impl ExtensionBuilder {
         Ok(())
     }
 
-    fn install_rust_wasm_target_if_needed(&self) -> Result<()> {
-        let rustc_output = util::command::new_std_command("rustc")
+    async fn install_rust_wasm_target_if_needed(&self) -> Result<()> {
+        let rustc_output = util::command::new_smol_command("rustc")
             .arg("--print")
             .arg("sysroot")
             .output()
+            .await
             .context("failed to run rustc")?;
         if !rustc_output.status.success() {
             bail!(
@@ -374,11 +383,12 @@ impl ExtensionBuilder {
             return Ok(());
         }
 
-        let output = util::command::new_std_command("rustup")
+        let output = util::command::new_smol_command("rustup")
             .args(["target", "add", RUST_TARGET])
             .stderr(Stdio::piped())
             .stdout(Stdio::inherit())
             .output()
+            .await
             .context("failed to run `rustup target add`")?;
         if !output.status.success() {
             bail!(