Merge branch 'master' into lint-attr-fix

3 files changed, 79 insertions, 29 deletions

diff --git a/src/libstd/sys/windows/c.rs b/src/libstd/sys/windows/c.rs
index d1c404195bc..dc7b2fc9a6b 100644
--- a/src/libstd/sys/windows/c.rs
+++ b/src/libstd/sys/windows/c.rs
@@ -265,6 +265,7 @@ pub const FILE_CURRENT: DWORD = 1;
 pub const FILE_END: DWORD = 2;
 
 pub const WAIT_OBJECT_0: DWORD = 0x00000000;
+pub const WAIT_TIMEOUT: DWORD = 258;
 
 #[cfg(target_env = "msvc")]
 pub const MAX_SYM_NAME: usize = 2000;
@@ -282,6 +283,7 @@ pub const EXCEPTION_STACK_OVERFLOW: DWORD = 0xc00000fd;
 pub const EXCEPTION_MAXIMUM_PARAMETERS: usize = 15;
 
 pub const PIPE_ACCESS_INBOUND: DWORD = 0x00000001;
+pub const PIPE_ACCESS_OUTBOUND: DWORD = 0x00000002;
 pub const FILE_FLAG_FIRST_PIPE_INSTANCE: DWORD = 0x00080000;
 pub const FILE_FLAG_OVERLAPPED: DWORD = 0x40000000;
 pub const PIPE_WAIT: DWORD = 0x00000000;
diff --git a/src/libstd/sys/windows/pipe.rs b/src/libstd/sys/windows/pipe.rs
index 1eb17305476..8073473f7ff 100644
--- a/src/libstd/sys/windows/pipe.rs
+++ b/src/libstd/sys/windows/pipe.rs
@@ -29,18 +29,43 @@ pub struct AnonPipe {
     inner: Handle,
 }
 
-pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
+pub struct Pipes {
+    pub ours: AnonPipe,
+    pub theirs: AnonPipe,
+}
+
+/// Although this looks similar to `anon_pipe` in the Unix module it's actually
+/// subtly different. Here we'll return two pipes in the `Pipes` return value,
+/// but one is intended for "us" where as the other is intended for "someone
+/// else".
+///
+/// Currently the only use case for this function is pipes for stdio on
+/// processes in the standard library, so "ours" is the one that'll stay in our
+/// process whereas "theirs" will be inherited to a child.
+///
+/// The ours/theirs pipes are *not* specifically readable or writable. Each
+/// one only supports a read or a write, but which is which depends on the
+/// boolean flag given. If `ours_readable` is true then `ours` is readable where
+/// `theirs` is writable. Conversely if `ours_readable` is false then `ours` is
+/// writable where `theirs` is readable.
+///
+/// Also note that the `ours` pipe is always a handle opened up in overlapped
+/// mode. This means that technically speaking it should only ever be used
+/// with `OVERLAPPED` instances, but also works out ok if it's only ever used
+/// once at a time (which we do indeed guarantee).
+pub fn anon_pipe(ours_readable: bool) -> io::Result<Pipes> {
     // Note that we specifically do *not* use `CreatePipe` here because
     // unfortunately the anonymous pipes returned do not support overlapped
-    // operations.
-    //
-    // Instead, we create a "hopefully unique" name and create a named pipe
-    // which has overlapped operations enabled.
+    // operations. Instead, we create a "hopefully unique" name and create a
+    // named pipe which has overlapped operations enabled.
     //
-    // Once we do this, we connect do it as usual via `CreateFileW`, and then we
-    // return those reader/writer halves.
+    // Once we do this, we connect do it as usual via `CreateFileW`, and then
+    // we return those reader/writer halves. Note that the `ours` pipe return
+    // value is always the named pipe, whereas `theirs` is just the normal file.
+    // This should hopefully shield us from child processes which assume their
+    // stdout is a named pipe, which would indeed be odd!
     unsafe {
-        let reader;
+        let ours;
         let mut name;
         let mut tries = 0;
         let mut reject_remote_clients_flag = c::PIPE_REJECT_REMOTE_CLIENTS;
@@ -54,11 +79,16 @@ pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
                                   .encode_wide()
                                   .chain(Some(0))
                                   .collect::<Vec<_>>();
+            let mut flags = c::FILE_FLAG_FIRST_PIPE_INSTANCE |
+                c::FILE_FLAG_OVERLAPPED;
+            if ours_readable {
+                flags |= c::PIPE_ACCESS_INBOUND;
+            } else {
+                flags |= c::PIPE_ACCESS_OUTBOUND;
+            }
 
             let handle = c::CreateNamedPipeW(wide_name.as_ptr(),
-                                             c::PIPE_ACCESS_INBOUND |
-                                             c::FILE_FLAG_FIRST_PIPE_INSTANCE |
-                                             c::FILE_FLAG_OVERLAPPED,
+                                             flags,
                                              c::PIPE_TYPE_BYTE |
                                              c::PIPE_READMODE_BYTE |
                                              c::PIPE_WAIT |
@@ -101,21 +131,28 @@ pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
                 }
                 return Err(err)
             }
-            reader = Handle::new(handle);
+            ours = Handle::new(handle);
             break
         }
 
-        // Connect to the named pipe we just created in write-only mode (also
-        // overlapped for async I/O below).
+        // Connect to the named pipe we just created. This handle is going to be
+        // returned in `theirs`, so if `ours` is readable we want this to be
+        // writable, otherwise if `ours` is writable we want this to be
+        // readable.
+        //
+        // Additionally we don't enable overlapped mode on this because most
+        // client processes aren't enabled to work with that.
         let mut opts = OpenOptions::new();
-        opts.write(true);
-        opts.read(false);
+        opts.write(ours_readable);
+        opts.read(!ours_readable);
         opts.share_mode(0);
-        opts.attributes(c::FILE_FLAG_OVERLAPPED);
-        let writer = File::open(Path::new(&name), &opts)?;
-        let writer = AnonPipe { inner: writer.into_handle() };
+        let theirs = File::open(Path::new(&name), &opts)?;
+        let theirs = AnonPipe { inner: theirs.into_handle() };
 
-        Ok((AnonPipe { inner: reader }, AnonPipe { inner: writer.into_handle() }))
+        Ok(Pipes {
+            ours: AnonPipe { inner: ours },
+            theirs: AnonPipe { inner: theirs.into_handle() },
+        })
     }
 }
 
diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs
index 969de6b85a6..d2ad81023e7 100644
--- a/src/libstd/sys/windows/process.rs
+++ b/src/libstd/sys/windows/process.rs
@@ -264,19 +264,15 @@ impl Stdio {
             }
 
             Stdio::MakePipe => {
-                let (reader, writer) = pipe::anon_pipe()?;
-                let (ours, theirs) = if stdio_id == c::STD_INPUT_HANDLE {
-                    (writer, reader)
-                } else {
-                    (reader, writer)
-                };
-                *pipe = Some(ours);
+                let ours_readable = stdio_id != c::STD_INPUT_HANDLE;
+                let pipes = pipe::anon_pipe(ours_readable)?;
+                *pipe = Some(pipes.ours);
                 cvt(unsafe {
-                    c::SetHandleInformation(theirs.handle().raw(),
+                    c::SetHandleInformation(pipes.theirs.handle().raw(),
                                             c::HANDLE_FLAG_INHERIT,
                                             c::HANDLE_FLAG_INHERIT)
                 })?;
-                Ok(theirs.into_handle())
+                Ok(pipes.theirs.into_handle())
             }
 
             Stdio::Handle(ref handle) => {
@@ -344,6 +340,21 @@ impl Process {
         }
     }
 
+    pub fn try_wait(&mut self) -> io::Result<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 Err(io::Error::last_os_error()),
+            }
+            let mut status = 0;
+            cvt(c::GetExitCodeProcess(self.handle.raw(), &mut status))?;
+            Ok(ExitStatus(status))
+        }
+    }
+
     pub fn handle(&self) -> &Handle { &self.handle }
 
     pub fn into_handle(self) -> Handle { self.handle }