diff --git a/Cargo.toml b/Cargo.toml index 7d99d76..f703343 100644 --- a/Cargo.toml +++ b/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"] } diff --git a/src/windows.rs b/src/windows.rs index 45dd1b1..c931d39 100644 --- a/src/windows.rs +++ b/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(wrapper: T) -> io::Result { - 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 {