Skip to content

Read of asynchronous pipe without synchronization #95411

Closed
@ChrisDenton

Description

@ChrisDenton

In src/sys/windows/pipe.rs a pipe is opened for asynchronous access (aka "overlapped mode").

/// Also note that the `ours` pipe is always a handle opened up in overlapped
/// mode. This means that technically speaking it should only ever be used
/// 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> {

However the read method for pipes is:

pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
self.inner.read(buf)
}

Where inner is a Handle whose read method does not wait for the buffer to be filled:

pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
let mut read = 0;
let len = cmp::min(buf.len(), <c::DWORD>::MAX as usize) as c::DWORD;
let res = cvt(unsafe {
c::ReadFile(
self.as_handle(),
buf.as_mut_ptr() as c::LPVOID,
len,
&mut read,
ptr::null_mut(),
)
});
match res {
Ok(_) => Ok(read as usize),
// The special treatment of BrokenPipe is to deal with Windows
// pipe semantics, which yields this error when *reading* from
// a pipe after the other end has closed; we interpret that as
// EOF on the pipe.
Err(ref e) if e.kind() == ErrorKind::BrokenPipe => Ok(0),
Err(e) => Err(e),
}
}

As far as I know this pipe is only used for process::ChildStd* so we (probably) get away with it in typical usage. Also the read2 method below this does the right thing by synchronizing reads.

Found while investigating #81357

@rustbot label +T-libs +O-windows

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: This is a bug.O-windowsOperating system: WindowsT-libsRelevant to the library team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions