Skip to content

Commit

Permalink
make Child::try_wait return io::Result<Option<ExitStatus>>
Browse files Browse the repository at this point in the history
This is much nicer for callers who want to short-circuit real I/O errors
with `?`, because they can write this

    if let Some(status) = foo.try_wait()? {
        ...
    } else {
        ...
    }

instead of this

    match foo.try_wait() {
        Ok(status) => {
            ...
        }
        Err(err) if err.kind() == io::ErrorKind::WouldBlock => {
            ...
        }
        Err(err) => return Err(err),
    }

The original design of `try_wait` was patterned after the `Read` and
`Write` traits, which support both blocking and non-blocking
implementations in a single API. But since `try_wait` is never blocking,
it makes sense to optimize for the non-blocking case.

Tracking issue: rust-lang#38903
  • Loading branch information
oconnor663 committed Feb 7, 2017
1 parent c49d102 commit 2a345bb
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 32 deletions.
15 changes: 7 additions & 8 deletions src/libstd/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -844,9 +844,9 @@ impl Child {
/// guaranteed to repeatedly return a successful exit status so long as the
/// child has already exited.
///
/// If the child has exited, then `Ok(status)` is returned. If the exit
/// status is not available at this time then an error is returned with the
/// error kind `WouldBlock`. If an error occurs, then that error is returned.
/// If the child has exited, then `Ok(Some(status))` is returned. If the
/// exit status is not available at this time then `Ok(None)` is returned.
/// If an error occurs, then that error is returned.
///
/// Note that unlike `wait`, this function will not attempt to drop stdin.
///
Expand All @@ -857,14 +857,13 @@ impl Child {
/// ```no_run
/// #![feature(process_try_wait)]
///
/// use std::io;
/// use std::process::Command;
///
/// let mut child = Command::new("ls").spawn().unwrap();
///
/// match child.try_wait() {
/// Ok(status) => println!("exited with: {}", status),
/// Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {
/// Ok(Some(status)) => println!("exited with: {}", status),
/// Ok(None) => {
/// println!("status not ready yet, let's really wait");
/// let res = child.wait();
/// println!("result: {:?}", res);
Expand All @@ -873,8 +872,8 @@ impl Child {
/// }
/// ```
#[unstable(feature = "process_try_wait", issue = "38903")]
pub fn try_wait(&mut self) -> io::Result<ExitStatus> {
self.handle.try_wait().map(ExitStatus)
pub fn try_wait(&mut self) -> io::Result<Option<ExitStatus>> {
Ok(self.handle.try_wait()?.map(ExitStatus))
}

/// Simultaneously waits for the child to exit and collect all remaining
Expand Down
8 changes: 4 additions & 4 deletions src/libstd/sys/redox/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,17 +502,17 @@ impl Process {
Ok(ExitStatus(status as i32))
}

pub fn try_wait(&mut self) -> io::Result<ExitStatus> {
pub fn try_wait(&mut self) -> io::Result<Option<ExitStatus>> {
if let Some(status) = self.status {
return Ok(status)
return Ok(Some(status))
}
let mut status = 0;
let pid = cvt(syscall::waitpid(self.pid, &mut status, syscall::WNOHANG))?;
if pid == 0 {
Err(io::Error::from_raw_os_error(syscall::EWOULDBLOCK))
Ok(None)
} else {
self.status = Some(ExitStatus(status as i32));
Ok(ExitStatus(status as i32))
Ok(Some(ExitStatus(status as i32)))
}
}
}
6 changes: 3 additions & 3 deletions src/libstd/sys/unix/process/process_fuchsia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl Process {
Ok(ExitStatus::new(proc_info.rec.return_code))
}

pub fn try_wait(&mut self) -> io::Result<ExitStatus> {
pub fn try_wait(&mut self) -> io::Result<Option<ExitStatus>> {
use default::Default;
use sys::process::magenta::*;

Expand All @@ -179,7 +179,7 @@ impl Process {
match status {
0 => { }, // Success
x if x == ERR_TIMED_OUT => {
return Err(io::Error::from(io::ErrorKind::WouldBlock));
return Ok(None);
},
_ => { panic!("Failed to wait on process handle: {}", status); },
}
Expand All @@ -192,7 +192,7 @@ impl Process {
return Err(io::Error::new(io::ErrorKind::InvalidData,
"Failed to get exit status of process"));
}
Ok(ExitStatus::new(proc_info.rec.return_code))
Ok(Some(ExitStatus::new(proc_info.rec.return_code)))
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/libstd/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,19 +249,19 @@ impl Process {
Ok(ExitStatus::new(status))
}

pub fn try_wait(&mut self) -> io::Result<ExitStatus> {
pub fn try_wait(&mut self) -> io::Result<Option<ExitStatus>> {
if let Some(status) = self.status {
return Ok(status)
return Ok(Some(status))
}
let mut status = 0 as c_int;
let pid = cvt(unsafe {
libc::waitpid(self.pid, &mut status, libc::WNOHANG)
})?;
if pid == 0 {
Err(io::Error::from_raw_os_error(libc::EWOULDBLOCK))
Ok(None)
} else {
self.status = Some(ExitStatus::new(status));
Ok(ExitStatus::new(status))
Ok(Some(ExitStatus::new(status)))
}
}
}
6 changes: 3 additions & 3 deletions src/libstd/sys/windows/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,18 +340,18 @@ impl Process {
}
}

pub fn try_wait(&mut self) -> io::Result<ExitStatus> {
pub fn try_wait(&mut self) -> io::Result<Option<ExitStatus>> {
unsafe {
match c::WaitForSingleObject(self.handle.raw(), 0) {
c::WAIT_OBJECT_0 => {}
c::WAIT_TIMEOUT => {
return Err(io::Error::from_raw_os_error(c::WSAEWOULDBLOCK))
return Ok(None);
}
_ => return Err(io::Error::last_os_error()),
}
let mut status = 0;
cvt(c::GetExitCodeProcess(self.handle.raw(), &mut status))?;
Ok(ExitStatus(status))
Ok(Some(ExitStatus(status)))
}
}

Expand Down
19 changes: 9 additions & 10 deletions src/test/run-pass/try-wait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#![feature(process_try_wait)]

use std::env;
use std::io;
use std::process::Command;
use std::thread;
use std::time::Duration;
Expand All @@ -32,17 +31,17 @@ fn main() {
.arg("sleep")
.spawn()
.unwrap();
let err = me.try_wait().unwrap_err();
assert_eq!(err.kind(), io::ErrorKind::WouldBlock);
let err = me.try_wait().unwrap_err();
assert_eq!(err.kind(), io::ErrorKind::WouldBlock);
let maybe_status = me.try_wait().unwrap();
assert!(maybe_status.is_none());
let maybe_status = me.try_wait().unwrap();
assert!(maybe_status.is_none());

me.kill().unwrap();
me.wait().unwrap();

let status = me.try_wait().unwrap();
let status = me.try_wait().unwrap().unwrap();
assert!(!status.success());
let status = me.try_wait().unwrap();
let status = me.try_wait().unwrap().unwrap();
assert!(!status.success());

let mut me = Command::new(env::current_exe().unwrap())
Expand All @@ -51,17 +50,17 @@ fn main() {
.unwrap();
loop {
match me.try_wait() {
Ok(res) => {
Ok(Some(res)) => {
assert!(res.success());
break
}
Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {
Ok(None) => {
thread::sleep(Duration::from_millis(1));
}
Err(e) => panic!("error in try_wait: {}", e),
}
}

let status = me.try_wait().unwrap();
let status = me.try_wait().unwrap().unwrap();
assert!(status.success());
}

0 comments on commit 2a345bb

Please sign in to comment.