Skip to content

Commit

Permalink
Auto merge of #112594 - ChrisDenton:process=-kill, r=Amanieu
Browse files Browse the repository at this point in the history
Return `Ok` on kill if process has already exited

This will require an FCP from `@rust-lang/libs-api.`

Fixes #112423. See that issue for more details.
  • Loading branch information
bors committed Jul 5, 2023
2 parents 99f7d36 + 9e5f61f commit dfe0683
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 14 deletions.
6 changes: 3 additions & 3 deletions library/std/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1904,8 +1904,8 @@ impl FromInner<imp::ExitCode> for ExitCode {
}

impl Child {
/// Forces the child process to exit. If the child has already exited, an [`InvalidInput`]
/// error is returned.
/// Forces the child process to exit. If the child has already exited, `Ok(())`
/// is returned.
///
/// The mapping to [`ErrorKind`]s is not part of the compatibility contract of the function.
///
Expand All @@ -1920,7 +1920,7 @@ impl Child {
///
/// let mut command = Command::new("yes");
/// if let Ok(mut child) = command.spawn() {
/// child.kill().expect("command wasn't running");
/// child.kill().expect("command couldn't be killed");
/// } else {
/// println!("yes command didn't start");
/// }
Expand Down
15 changes: 15 additions & 0 deletions library/std/src/process/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,3 +582,18 @@ fn run_canonical_bat_script() {
assert!(output.status.success());
assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "Hello, fellow Rustaceans!");
}

#[test]
fn terminate_exited_process() {
let mut cmd = if cfg!(target_os = "android") {
let mut p = shell_cmd();
p.args(&["-c", "true"]);
p
} else {
known_command()
};
let mut p = cmd.stdout(Stdio::null()).spawn().unwrap();
p.wait().unwrap();
assert!(p.kill().is_ok());
assert!(p.kill().is_ok());
}
7 changes: 2 additions & 5 deletions library/std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,12 +762,9 @@ impl Process {
pub fn kill(&mut self) -> io::Result<()> {
// If we've already waited on this process then the pid can be recycled
// and used for another process, and we probably shouldn't be killing
// random processes, so just return an error.
// random processes, so return Ok because the process has exited already.
if self.status.is_some() {
Err(io::const_io_error!(
ErrorKind::InvalidInput,
"invalid argument: can't kill an exited process",
))
Ok(())
} else {
cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop)
}
Expand Down
7 changes: 2 additions & 5 deletions library/std/src/sys/unix/process/process_vxworks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,9 @@ impl Process {
pub fn kill(&mut self) -> io::Result<()> {
// If we've already waited on this process then the pid can be recycled
// and used for another process, and we probably shouldn't be killing
// random processes, so just return an error.
// random processes, so return Ok because the process has exited already.
if self.status.is_some() {
Err(io::const_io_error!(
ErrorKind::InvalidInput,
"invalid argument: can't kill an exited process",
))
Ok(())
} else {
cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop)
}
Expand Down
11 changes: 10 additions & 1 deletion library/std/src/sys/windows/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,16 @@ pub struct Process {

impl Process {
pub fn kill(&mut self) -> io::Result<()> {
cvt(unsafe { c::TerminateProcess(self.handle.as_raw_handle(), 1) })?;
let result = unsafe { c::TerminateProcess(self.handle.as_raw_handle(), 1) };
if result == c::FALSE {
let error = unsafe { c::GetLastError() };
// TerminateProcess returns ERROR_ACCESS_DENIED if the process has already been
// terminated (by us, or for any other reason). So check if the process was actually
// terminated, and if so, do not return an error.
if error != c::ERROR_ACCESS_DENIED || self.try_wait().is_err() {
return Err(crate::io::Error::from_raw_os_error(error as i32));
}
}
Ok(())
}

Expand Down

0 comments on commit dfe0683

Please sign in to comment.