diff options
| author | Mark Rousskov <mark.simulacrum@gmail.com> | 2022-05-17 18:40:35 -0400 |
|---|---|---|
| committer | Mark Rousskov <mark.simulacrum@gmail.com> | 2022-05-17 18:40:35 -0400 |
| commit | 79256566a9da361793eba86ff1a40caf1560903d (patch) | |
| tree | 57036f4ce76b0e8f11213cd3b829dbea985ba212 | |
| parent | fba04fdb141461aefb58d0ffb15857cfc146758f (diff) | |
| download | rust-79256566a9da361793eba86ff1a40caf1560903d.tar.gz rust-79256566a9da361793eba86ff1a40caf1560903d.zip | |
Revert "Windows: Make stdin pipes synchronous"
This reverts commit 949b978ec9d63b0eea23d89bad16c6f022ac34a3.
| -rw-r--r-- | library/std/src/sys/windows/c.rs | 6 | ||||
| -rw-r--r-- | library/std/src/sys/windows/pipe.rs | 90 | ||||
| -rw-r--r-- | library/std/src/sys/windows/process.rs | 20 |
3 files changed, 31 insertions, 85 deletions
diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index 0692da1d795..5f14edaf067 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -1022,12 +1022,6 @@ extern "system" { bWaitAll: BOOL, dwMilliseconds: DWORD, ) -> DWORD; - pub fn CreatePipe( - hReadPipe: *mut HANDLE, - hWritePipe: *mut HANDLE, - lpPipeAttributes: *const SECURITY_ATTRIBUTES, - nSize: DWORD, - ) -> BOOL; pub fn CreateNamedPipeW( lpName: LPCWSTR, dwOpenMode: DWORD, diff --git a/library/std/src/sys/windows/pipe.rs b/library/std/src/sys/windows/pipe.rs index 928bf2439c3..013c776c476 100644 --- a/library/std/src/sys/windows/pipe.rs +++ b/library/std/src/sys/windows/pipe.rs @@ -18,20 +18,13 @@ use crate::sys_common::IntoInner; // Anonymous pipes //////////////////////////////////////////////////////////////////////////////// -// A 64kb pipe capacity is the same as a typical Linux default. -const PIPE_BUFFER_CAPACITY: u32 = 64 * 1024; - -pub enum AnonPipe { - Sync(Handle), - Async(Handle), +pub struct AnonPipe { + inner: Handle, } impl IntoInner<Handle> for AnonPipe { fn into_inner(self) -> Handle { - match self { - Self::Sync(handle) => handle, - Self::Async(handle) => handle, - } + self.inner } } @@ -39,35 +32,6 @@ pub struct Pipes { pub ours: AnonPipe, pub theirs: AnonPipe, } -impl Pipes { - /// Create a new pair of pipes where both pipes are synchronous. - /// - /// These must not be used asynchronously. - pub fn new_synchronous( - ours_readable: bool, - their_handle_inheritable: bool, - ) -> io::Result<Self> { - unsafe { - // If `CreatePipe` succeeds, these will be our pipes. - let mut read = ptr::null_mut(); - let mut write = ptr::null_mut(); - - if c::CreatePipe(&mut read, &mut write, ptr::null(), PIPE_BUFFER_CAPACITY) == 0 { - Err(io::Error::last_os_error()) - } else { - let (ours, theirs) = if ours_readable { (read, write) } else { (write, read) }; - let ours = Handle::from_raw_handle(ours); - let theirs = Handle::from_raw_handle(theirs); - - if their_handle_inheritable { - theirs.set_inheritable()?; - } - - Ok(Pipes { ours: AnonPipe::Sync(ours), theirs: AnonPipe::Sync(theirs) }) - } - } - } -} /// 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, @@ -89,6 +53,9 @@ impl Pipes { /// 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, their_handle_inheritable: bool) -> io::Result<Pipes> { + // A 64kb pipe capacity is the same as a typical Linux default. + const PIPE_BUFFER_CAPACITY: u32 = 64 * 1024; + // 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 @@ -189,9 +156,12 @@ pub fn anon_pipe(ours_readable: bool, their_handle_inheritable: bool) -> io::Res }; opts.security_attributes(&mut sa); let theirs = File::open(Path::new(&name), &opts)?; - let theirs = AnonPipe::Sync(theirs.into_inner()); + let theirs = AnonPipe { inner: theirs.into_inner() }; - Ok(Pipes { ours: AnonPipe::Async(ours), theirs }) + Ok(Pipes { + ours: AnonPipe { inner: ours }, + theirs: AnonPipe { inner: theirs.into_inner() }, + }) } } @@ -201,12 +171,12 @@ pub fn anon_pipe(ours_readable: bool, their_handle_inheritable: bool) -> io::Res /// This is achieved by creating a new set of pipes and spawning a thread that /// relays messages between the source and the synchronous pipe. pub fn spawn_pipe_relay( - source: &Handle, + source: &AnonPipe, ours_readable: bool, their_handle_inheritable: bool, ) -> io::Result<AnonPipe> { // We need this handle to live for the lifetime of the thread spawned below. - let source = AnonPipe::Async(source.duplicate(0, true, c::DUPLICATE_SAME_ACCESS)?); + let source = source.duplicate()?; // create a new pair of anon pipes. let Pipes { theirs, ours } = anon_pipe(ours_readable, their_handle_inheritable)?; @@ -257,24 +227,19 @@ type AlertableIoFn = unsafe extern "system" fn( impl AnonPipe { pub fn handle(&self) -> &Handle { - match self { - Self::Async(ref handle) => handle, - Self::Sync(ref handle) => handle, - } + &self.inner } pub fn into_handle(self) -> Handle { - self.into_inner() + self.inner + } + fn duplicate(&self) -> io::Result<Self> { + self.inner.duplicate(0, false, c::DUPLICATE_SAME_ACCESS).map(|inner| AnonPipe { inner }) } pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> { let result = unsafe { let len = crate::cmp::min(buf.len(), c::DWORD::MAX as usize) as c::DWORD; - match self { - Self::Sync(ref handle) => handle.read(buf), - Self::Async(_) => { - self.alertable_io_internal(c::ReadFileEx, buf.as_mut_ptr() as _, len) - } - } + self.alertable_io_internal(c::ReadFileEx, buf.as_mut_ptr() as _, len) }; match result { @@ -288,33 +253,28 @@ impl AnonPipe { } pub fn read_vectored(&self, bufs: &mut [IoSliceMut<'_>]) -> io::Result<usize> { - io::default_read_vectored(|buf| self.read(buf), bufs) + self.inner.read_vectored(bufs) } #[inline] pub fn is_read_vectored(&self) -> bool { - false + self.inner.is_read_vectored() } pub fn write(&self, buf: &[u8]) -> io::Result<usize> { unsafe { let len = crate::cmp::min(buf.len(), c::DWORD::MAX as usize) as c::DWORD; - match self { - Self::Sync(ref handle) => handle.write(buf), - Self::Async(_) => { - self.alertable_io_internal(c::WriteFileEx, buf.as_ptr() as _, len) - } - } + self.alertable_io_internal(c::WriteFileEx, buf.as_ptr() as _, len) } } pub fn write_vectored(&self, bufs: &[IoSlice<'_>]) -> io::Result<usize> { - io::default_write_vectored(|buf| self.write(buf), bufs) + self.inner.write_vectored(bufs) } #[inline] pub fn is_write_vectored(&self) -> bool { - false + self.inner.is_write_vectored() } /// Synchronizes asynchronous reads or writes using our anonymous pipe. @@ -386,7 +346,7 @@ impl AnonPipe { // Asynchronous read of the pipe. // If successful, `callback` will be called once it completes. - let result = io(self.handle().as_handle(), buf, len, &mut overlapped, callback); + let result = io(self.inner.as_handle(), buf, len, &mut overlapped, callback); if result == c::FALSE { // We can return here because the call failed. // After this we must not return until the I/O completes. diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index dd931688bec..9fd399f4ba1 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -23,7 +23,7 @@ use crate::sys::cvt; use crate::sys::fs::{File, OpenOptions}; use crate::sys::handle::Handle; use crate::sys::path; -use crate::sys::pipe::{self, AnonPipe, Pipes}; +use crate::sys::pipe::{self, AnonPipe}; use crate::sys::stdio; use crate::sys_common::mutex::StaticMutex; use crate::sys_common::process::{CommandEnv, CommandEnvs}; @@ -172,7 +172,7 @@ pub enum Stdio { Inherit, Null, MakePipe, - AsyncPipe(Handle), + Pipe(AnonPipe), Handle(Handle), } @@ -527,18 +527,13 @@ impl Stdio { }, Stdio::MakePipe => { - // If stdin then make synchronous - let pipes = if stdio_id == c::STD_INPUT_HANDLE { - Pipes::new_synchronous(false, true)? - } else { - pipe::anon_pipe(true, true)? - }; + let ours_readable = stdio_id != c::STD_INPUT_HANDLE; + let pipes = pipe::anon_pipe(ours_readable, true)?; *pipe = Some(pipes.ours); Ok(pipes.theirs.into_handle()) } - Stdio::AsyncPipe(ref source) => { - // We need to synchronize asynchronous pipes by using a pipe relay. + Stdio::Pipe(ref source) => { let ours_readable = stdio_id != c::STD_INPUT_HANDLE; pipe::spawn_pipe_relay(source, ours_readable, true).map(AnonPipe::into_handle) } @@ -567,10 +562,7 @@ impl Stdio { impl From<AnonPipe> for Stdio { fn from(pipe: AnonPipe) -> Stdio { - match pipe { - AnonPipe::Sync(handle) => Stdio::Handle(handle), - AnonPipe::Async(handle) => Stdio::AsyncPipe(handle), - } + Stdio::Pipe(pipe) } } |
