Skip to content

Commit

Permalink
fix a bug where cloned pipes were inheritable on Windows
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
oconnor663 committed Oct 11, 2019
1 parent 3b187a3 commit 3794cb7
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 5 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
40 changes: 36 additions & 4 deletions src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
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 RawHandle;
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)) }
}
}

impl IntoRawHandle for PipeReader {
Expand Down

0 comments on commit 3794cb7

Please sign in to comment.