From 2a345bbcc1e6332241883f784896ea93d2a7ccb3 Mon Sep 17 00:00:00 2001 From: Jack O'Connor Date: Fri, 3 Feb 2017 17:39:41 -0500 Subject: [PATCH] make Child::try_wait return io::Result> 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: https://github.com/rust-lang/rust/issues/38903 --- src/libstd/process.rs | 15 +++++++-------- src/libstd/sys/redox/process.rs | 8 ++++---- .../sys/unix/process/process_fuchsia.rs | 6 +++--- src/libstd/sys/unix/process/process_unix.rs | 8 ++++---- src/libstd/sys/windows/process.rs | 6 +++--- src/test/run-pass/try-wait.rs | 19 +++++++++---------- 6 files changed, 30 insertions(+), 32 deletions(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index c16b97ebda5e3..4ff35738b50fb 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -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. /// @@ -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); @@ -873,8 +872,8 @@ impl Child { /// } /// ``` #[unstable(feature = "process_try_wait", issue = "38903")] - pub fn try_wait(&mut self) -> io::Result { - self.handle.try_wait().map(ExitStatus) + pub fn try_wait(&mut self) -> io::Result> { + Ok(self.handle.try_wait()?.map(ExitStatus)) } /// Simultaneously waits for the child to exit and collect all remaining diff --git a/src/libstd/sys/redox/process.rs b/src/libstd/sys/redox/process.rs index 50dcd44b42e92..60dc03fcf47e2 100644 --- a/src/libstd/sys/redox/process.rs +++ b/src/libstd/sys/redox/process.rs @@ -502,17 +502,17 @@ impl Process { Ok(ExitStatus(status as i32)) } - pub fn try_wait(&mut self) -> io::Result { + pub fn try_wait(&mut self) -> io::Result> { 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))) } } } diff --git a/src/libstd/sys/unix/process/process_fuchsia.rs b/src/libstd/sys/unix/process/process_fuchsia.rs index 87acb0ed9b977..0bb2e0c1a83d4 100644 --- a/src/libstd/sys/unix/process/process_fuchsia.rs +++ b/src/libstd/sys/unix/process/process_fuchsia.rs @@ -165,7 +165,7 @@ impl Process { Ok(ExitStatus::new(proc_info.rec.return_code)) } - pub fn try_wait(&mut self) -> io::Result { + pub fn try_wait(&mut self) -> io::Result> { use default::Default; use sys::process::magenta::*; @@ -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); }, } @@ -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))) } } diff --git a/src/libstd/sys/unix/process/process_unix.rs b/src/libstd/sys/unix/process/process_unix.rs index 0dc1739c1a15a..bbc987209e300 100644 --- a/src/libstd/sys/unix/process/process_unix.rs +++ b/src/libstd/sys/unix/process/process_unix.rs @@ -249,19 +249,19 @@ impl Process { Ok(ExitStatus::new(status)) } - pub fn try_wait(&mut self) -> io::Result { + pub fn try_wait(&mut self) -> io::Result> { 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))) } } } diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index d2ad81023e7fe..1afb3728c9d72 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -340,18 +340,18 @@ impl Process { } } - pub fn try_wait(&mut self) -> io::Result { + pub fn try_wait(&mut self) -> io::Result> { 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))) } } diff --git a/src/test/run-pass/try-wait.rs b/src/test/run-pass/try-wait.rs index d9826373cceb0..be87b7b3c87e4 100644 --- a/src/test/run-pass/try-wait.rs +++ b/src/test/run-pass/try-wait.rs @@ -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; @@ -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()) @@ -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()); }