Skip to content

Commit b3dcd3f

Browse files
committed
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.
1 parent 3b187a3 commit b3dcd3f

File tree

1 file changed

+36
-3
lines changed

1 file changed

+36
-3
lines changed

src/windows.rs

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@ use std::io;
55
use std::mem::ManuallyDrop;
66
use std::os::windows::prelude::*;
77
use std::ptr;
8+
use winapi::shared::minwindef::BOOL;
89
use winapi::shared::ntdef::{HANDLE, PHANDLE};
10+
use winapi::um::handleapi::DuplicateHandle;
911
use winapi::um::namedpipeapi;
12+
use winapi::um::processthreadsapi::GetCurrentProcess;
13+
use winapi::um::winnt::DUPLICATE_SAME_ACCESS;
1014

1115
pub(crate) fn pipe() -> io::Result<(PipeReader, PipeWriter)> {
1216
let mut read_pipe: HANDLE = ptr::null_mut();
@@ -36,9 +40,38 @@ pub(crate) fn pipe() -> io::Result<(PipeReader, PipeWriter)> {
3640
}
3741

3842
pub(crate) fn dup<T: AsRawHandle>(wrapper: T) -> io::Result<File> {
39-
let handle = wrapper.as_raw_handle();
40-
let temp_file = ManuallyDrop::new(unsafe { File::from_raw_handle(handle) });
41-
temp_file.try_clone()
43+
// We rely on ("abuse") std::fs::File for a lot of descriptor/handle
44+
// operations. (For example, setting F_DUPFD_CLOEXEC on Unix is a
45+
// compatibility mess.) However, in the particular case of try_clone on
46+
// Windows, the standard library has a bug where duplicated handles end up
47+
// inheritable when they shouldn't be. See
48+
// https://github.com/rust-lang/rust/pull/65316. This leads to races where
49+
// child processes can inherit each other's handles, which tends to cause
50+
// deadlocks when the handle in question is a stdout pipe. To get that
51+
// right, we explicitly make the necessary system calls here, just like
52+
// libstd apart from that one flag.
53+
let source_handle = wrapper.as_raw_handle();
54+
let desired_access = 0; // Ignored because of DUPLICATE_SAME_ACCESS.
55+
let inherit_handle = false as BOOL; // <-- Libstd sets this to true!
56+
let options = DUPLICATE_SAME_ACCESS;
57+
let mut duplicated_handle: RawHandle = 0;
58+
let ret = unsafe {
59+
let current_process = GetCurrentProcess();
60+
c::DuplicateHandle(
61+
current_process,
62+
source_handle,
63+
current_process,
64+
&mut duplicated_handle,
65+
desired_access,
66+
inherit_handle,
67+
options,
68+
)
69+
};
70+
if ret == 0 {
71+
Err(io::Error::last_os_error())
72+
} else {
73+
unsafe { Ok(File::from_raw_handle(duplicated_handle)) }
74+
}
4275
}
4376

4477
impl IntoRawHandle for PipeReader {

0 commit comments

Comments
 (0)