diff --git a/library/std/src/process.rs b/library/std/src/process.rs index fb78e62834a07..78eb98f405b30 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -132,15 +132,25 @@ use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; /// /// # Warning /// +/// A `Child` which is simply dropped may continue running in parallel, +/// possibly even after the program which launched it exits. +/// If it inherited stdin/stdout, it may still read and write to them +/// later, causing confusion and disruption. +/// /// On some systems, calling [`wait`] or similar is necessary for the OS to /// release resources. A process that terminated but has not been waited on is /// still around as a "zombie". Leaving too many zombies around may exhaust /// global resources (for example process IDs). /// +/// Unless [`wait`] is called, any failure of the child will not be +/// visible. +/// /// The standard library does *not* automatically wait on child processes (not /// even if the `Child` is dropped), it is up to the application developer to do -/// so. As a consequence, dropping `Child` handles without waiting on them first -/// is not recommended in long-running applications. +/// so. +/// +/// For these reasons, dropping `Child` handles without waiting on them first +/// is usually incorrect. /// /// # Examples /// @@ -160,6 +170,7 @@ use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; /// /// [`wait`]: Child::wait #[stable(feature = "process", since = "1.0.0")] +#[must_use = "this Child should probably be `wait`ed, or `status` used instead of `spawn`"] pub struct Child { handle: imp::Process, @@ -483,7 +494,8 @@ impl fmt::Debug for ChildStderr { /// let mut list_dir = Command::new("ls"); /// /// // Execute `ls` in the current directory of the program. -/// list_dir.status().expect("process failed to execute"); +/// let status = list_dir.status().expect("process failed to execute"); +/// assert!(status.success()); /// /// println!(); /// @@ -491,9 +503,11 @@ impl fmt::Debug for ChildStderr { /// list_dir.current_dir("/"); /// /// // And then execute `ls` again but in the root directory. -/// list_dir.status().expect("process failed to execute"); +/// let status = list_dir.status().expect("process failed to execute"); +/// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] +#[must_use = "this Command does not do anything unless it is run, eg using `status` or `spawn`"] pub struct Command { inner: imp::Command, } @@ -525,9 +539,11 @@ impl Command { /// ```no_run /// use std::process::Command; /// - /// Command::new("sh") - /// .spawn() + /// let status = Command::new("sh") + /// .status() /// .expect("sh command failed to start"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn new>(program: S) -> Command { @@ -569,11 +585,13 @@ impl Command { /// ```no_run /// use std::process::Command; /// - /// Command::new("ls") + /// let status = Command::new("ls") /// .arg("-l") /// .arg("-a") - /// .spawn() + /// .status() /// .expect("ls command failed to start"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn arg>(&mut self, arg: S) -> &mut Command { @@ -599,10 +617,12 @@ impl Command { /// ```no_run /// use std::process::Command; /// - /// Command::new("ls") + /// let status = Command::new("ls") /// .args(&["-l", "-a"]) - /// .spawn() + /// .status() /// .expect("ls command failed to start"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn args(&mut self, args: I) -> &mut Command @@ -628,10 +648,12 @@ impl Command { /// ```no_run /// use std::process::Command; /// - /// Command::new("ls") + /// let status = Command::new("ls") /// .env("PATH", "/bin") - /// .spawn() + /// .status() /// .expect("ls command failed to start"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn env(&mut self, key: K, val: V) -> &mut Command @@ -659,13 +681,15 @@ impl Command { /// k == "TERM" || k == "TZ" || k == "LANG" || k == "PATH" /// ).collect(); /// - /// Command::new("printenv") + /// let status = Command::new("printenv") /// .stdin(Stdio::null()) /// .stdout(Stdio::inherit()) /// .env_clear() /// .envs(&filtered_env) - /// .spawn() + /// .status() /// .expect("printenv failed to start"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "command_envs", since = "1.19.0")] pub fn envs(&mut self, vars: I) -> &mut Command @@ -689,10 +713,12 @@ impl Command { /// ```no_run /// use std::process::Command; /// - /// Command::new("ls") + /// let status = Command::new("ls") /// .env_remove("PATH") - /// .spawn() + /// .status() /// .expect("ls command failed to start"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn env_remove>(&mut self, key: K) -> &mut Command { @@ -709,10 +735,12 @@ impl Command { /// ```no_run /// use std::process::Command; /// - /// Command::new("ls") + /// let status = Command::new("ls") /// .env_clear() - /// .spawn() + /// .status() /// .expect("ls command failed to start"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn env_clear(&mut self) -> &mut Command { @@ -737,10 +765,12 @@ impl Command { /// ```no_run /// use std::process::Command; /// - /// Command::new("ls") + /// let status = Command::new("ls") /// .current_dir("/bin") - /// .spawn() + /// .status() /// .expect("ls command failed to start"); + /// + /// assert!(status.success()); /// ``` /// /// [`canonicalize`]: crate::fs::canonicalize @@ -765,10 +795,12 @@ impl Command { /// ```no_run /// use std::process::{Command, Stdio}; /// - /// Command::new("ls") + /// let status = Command::new("ls") /// .stdin(Stdio::null()) - /// .spawn() + /// .status() /// .expect("ls command failed to start"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn stdin>(&mut self, cfg: T) -> &mut Command { @@ -791,10 +823,12 @@ impl Command { /// ```no_run /// use std::process::{Command, Stdio}; /// - /// Command::new("ls") + /// let status = Command::new("ls") /// .stdout(Stdio::null()) - /// .spawn() + /// .status() /// .expect("ls command failed to start"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn stdout>(&mut self, cfg: T) -> &mut Command { @@ -817,10 +851,12 @@ impl Command { /// ```no_run /// use std::process::{Command, Stdio}; /// - /// Command::new("ls") + /// let status = Command::new("ls") /// .stderr(Stdio::null()) - /// .spawn() + /// .status() /// .expect("ls command failed to start"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn stderr>(&mut self, cfg: T) -> &mut Command { @@ -832,6 +868,9 @@ impl Command { /// /// By default, stdin, stdout and stderr are inherited from the parent. /// + /// If you just want to run the command and wait for it to complete, consider using + /// [`Command::status`] instead. + /// /// # Examples /// /// Basic usage: @@ -839,9 +878,13 @@ impl Command { /// ```no_run /// use std::process::Command; /// - /// Command::new("ls") + /// let status = Command::new("ls") /// .spawn() - /// .expect("ls command failed to start"); + /// .expect("ls command failed to start") + /// .wait() + /// .expect("failed to wait for child"); + /// + /// assert!(status.success()); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn spawn(&mut self) -> io::Result { @@ -1373,6 +1416,7 @@ impl From for Stdio { /// [`wait`]: Child::wait #[derive(PartialEq, Eq, Clone, Copy, Debug)] #[stable(feature = "process", since = "1.0.0")] +#[must_use = "this ExitStatus might represent an error that should be checked and handled"] pub struct ExitStatus(imp::ExitStatus); impl ExitStatus { @@ -1557,8 +1601,8 @@ impl Child { /// /// let mut command = Command::new("ls"); /// if let Ok(mut child) = command.spawn() { - /// child.wait().expect("command wasn't running"); - /// println!("Child has finished its execution!"); + /// let status = child.wait().expect("command wasn't running"); + /// println!("Child has finished its execution, status = {}!", status); /// } else { /// println!("ls command didn't start"); /// } diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs index 3694bdbf67054..7cdd4d647c5fc 100644 --- a/src/bootstrap/bin/rustc.rs +++ b/src/bootstrap/bin/rustc.rs @@ -179,7 +179,10 @@ fn main() { } if let Some(mut on_fail) = on_fail { - on_fail.status().expect("Could not run the on_fail command"); + let status = on_fail.status().expect("Could not run the on_fail command"); + if !status.success() { + eprintln!("\nwarning: the on-fail command also failed: {}\n", status); + } } // Preserve the exit code. In case of signal, exit with 0xfe since it's diff --git a/src/test/ui/fds-are-cloexec.rs b/src/test/ui/fds-are-cloexec.rs index 4482b7032a75a..465eef61a3621 100644 --- a/src/test/ui/fds-are-cloexec.rs +++ b/src/test/ui/fds-are-cloexec.rs @@ -66,7 +66,9 @@ fn parent() { .status() .unwrap(); assert!(status.success()); - child.wait().unwrap(); + + let status = child.wait().unwrap(); + assert!(status.success()); } fn child(args: &[String]) { diff --git a/src/test/ui/try-wait.rs b/src/test/ui/try-wait.rs index 692197210b15f..cdc7fe44a7d2e 100644 --- a/src/test/ui/try-wait.rs +++ b/src/test/ui/try-wait.rs @@ -31,7 +31,8 @@ fn main() { assert!(maybe_status.is_none()); me.kill().unwrap(); - me.wait().unwrap(); + let status = me.wait().unwrap(); + assert!(!status.success()); let status = me.try_wait().unwrap().unwrap(); assert!(!status.success()); diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 5608ff98417cd..f78975b64584e 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -869,17 +869,23 @@ impl<'test> TestCx<'test> { let adb_path = &self.config.adb_path; - Command::new(adb_path) + let status = Command::new(adb_path) .arg("push") .arg(&exe_file) .arg(&self.config.adb_test_dir) .status() .unwrap_or_else(|_| panic!("failed to exec `{:?}`", adb_path)); + if !status.success() { + panic!("Program failed `{:}`", adb_path); + } - Command::new(adb_path) + let status = Command::new(adb_path) .args(&["forward", "tcp:5039", "tcp:5039"]) .status() .unwrap_or_else(|_| panic!("failed to exec `{:?}`", adb_path)); + if !status.success() { + panic!("Program failed `{:}`", adb_path); + } let adb_arg = format!( "export LD_LIBRARY_PATH={}; \