Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
]
10 changes: 4 additions & 6 deletions crates/ide/src/expand_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
2 changes: 1 addition & 1 deletion crates/ide/src/syntax_highlighting/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
Expand Down
22 changes: 11 additions & 11 deletions crates/proc-macro-api/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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())
},
Expand Down Expand Up @@ -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))
Expand All @@ -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 {
Expand All @@ -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)
}
21 changes: 18 additions & 3 deletions crates/project-model/src/cargo_workspace.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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:?}`"));

Expand Down Expand Up @@ -836,3 +836,18 @@ 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_lossy(&output.stderr).into_owned(),
}
.into());
}
let stdout = from_utf8(&output.stdout)?
.lines()
.find(|line| line.starts_with('{'))
.ok_or(cargo_metadata::Error::NoJson)?;
Ok(MetadataCommand::parse(stdout)?)
}
2 changes: 1 addition & 1 deletion crates/project-model/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() => {
Expand Down
4 changes: 3 additions & 1 deletion crates/project-model/src/sysroot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
1 change: 0 additions & 1 deletion crates/rust-analyzer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions crates/rust-analyzer/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 3 additions & 6 deletions crates/rust-analyzer/src/bin/rustc_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
40 changes: 10 additions & 30 deletions crates/rust-analyzer/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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 =
Expand All @@ -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(())
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
9 changes: 3 additions & 6 deletions crates/rust-analyzer/src/handlers/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?
};
Expand Down
8 changes: 4 additions & 4 deletions crates/rust-analyzer/tests/slow-tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 9 additions & 1 deletion crates/stdx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading