fix a bug where cloned pipes were inheritable on Windows

There is a bug (I think it's a bug?) in libstd which makes
File::try_clone return inheritable handles. We need to work around that
by making the same system calls explictly, but with the inheritable flag
set to false. I've filed a fix against libstd
(rust-lang/rust#65316), but even if that lands
we'll need to retain this workaround for all the past versions of Rust
that have the bug.

Author: oconnor663

Cargo.toml

@@ -13,4 +13,4 @@ edition = "2018"
 libc = "0.2.62"
 
 [target.'cfg(windows)'.dependencies]
-winapi = { version = "0.3.5", features = ["namedpipeapi"] }
+winapi = { version = "0.3.5", features = ["handleapi", "namedpipeapi", "processthreadsapi", "winnt"] }

src/lib.rs

@@ -340,6 +340,13 @@ mod tests {
         let (input_reader, mut input_writer) = crate::pipe().unwrap();
         let (mut output_reader, output_writer) = crate::pipe().unwrap();
 
+        // Create a bunch of duplicated copies, which we'll close later. This
+        // tests that duplication preserves non-inheritability.
+        let ir_dup = input_reader.try_clone().unwrap();
+        let iw_dup = input_writer.try_clone().unwrap();
+        let or_dup = output_reader.try_clone().unwrap();
+        let ow_dup = output_writer.try_clone().unwrap();
+
         // Spawn the child. Note that this temporary Command object takes
         // ownership of our copies of the child's stdin and stdout, and then
         // closes them immediately when it drops. That stops us from blocking
@@ -351,6 +358,12 @@ mod tests {
             .spawn()
             .unwrap();
 
+        // Drop all the dups now that the child is spawned.
+        drop(ir_dup);
+        drop(iw_dup);
+        drop(or_dup);
+        drop(ow_dup);
+
         // Write to the child's stdin. This is a small write, so it shouldn't
         // block.
         input_writer.write_all(b"hello").unwrap();

src/windows.rs

@@ -2,11 +2,14 @@ use crate::PipeReader;
 use crate::PipeWriter;
 use std::fs::File;
 use std::io;
-use std::mem::ManuallyDrop;
 use std::os::windows::prelude::*;
 use std::ptr;
+use winapi::shared::minwindef::BOOL;
 use winapi::shared::ntdef::{HANDLE, PHANDLE};
+use winapi::um::handleapi::DuplicateHandle;
 use winapi::um::namedpipeapi;
+use winapi::um::processthreadsapi::GetCurrentProcess;
+use winapi::um::winnt::DUPLICATE_SAME_ACCESS;
 
 pub(crate) fn pipe() -> io::Result<(PipeReader, PipeWriter)> {
     let mut read_pipe: HANDLE = ptr::null_mut();
@@ -36,9 +39,38 @@ pub(crate) fn pipe() -> io::Result<(PipeReader, PipeWriter)> {
 }
 
 pub(crate) fn dup<T: AsRawHandle>(wrapper: T) -> io::Result<File> {
-    let handle = wrapper.as_raw_handle();
-    let temp_file = ManuallyDrop::new(unsafe { File::from_raw_handle(handle) });
-    temp_file.try_clone()
+    // We rely on ("abuse") std::fs::File for a lot of descriptor/handle
+    // operations. (For example, setting F_DUPFD_CLOEXEC on Unix is a
+    // compatibility mess.) However, in the particular case of try_clone on
+    // Windows, the standard library has a bug where duplicated handles end up
+    // inheritable when they shouldn't be. See
+    // https://github.com/rust-lang/rust/pull/65316. This leads to races where
+    // child processes can inherit each other's handles, which tends to cause
+    // deadlocks when the handle in question is a stdout pipe. To get that
+    // right, we explicitly make the necessary system calls here, just like
+    // libstd apart from that one flag.
+    let source_handle = wrapper.as_raw_handle() as HANDLE;
+    let desired_access = 0; // Ignored because of DUPLICATE_SAME_ACCESS.
+    let inherit_handle = false as BOOL; // <-- Libstd sets this to true!
+    let options = DUPLICATE_SAME_ACCESS;
+    let mut duplicated_handle = 0 as HANDLE;
+    let ret = unsafe {
+        let current_process = GetCurrentProcess();
+        DuplicateHandle(
+            current_process,
+            source_handle,
+            current_process,
+            &mut duplicated_handle,
+            desired_access,
+            inherit_handle,
+            options,
+        )
+    };
+    if ret == 0 {
+        Err(io::Error::last_os_error())
+    } else {
+        unsafe { Ok(File::from_raw_handle(duplicated_handle as RawHandle)) }
+    }
 }
 
 impl IntoRawHandle for PipeReader {