Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make Child::try_wait return io::Result<Option<ExitStatus>> #39512

Merged
merged 1 commit into from
Feb 8, 2017
Merged
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
make Child::try_wait return io::Result<Option<ExitStatus>>
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: #38903
  • Loading branch information
oconnor663 committed Feb 7, 2017
commit 2a345bbcc1e6332241883f784896ea93d2a7ccb3
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());
}