Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a BorrowedFd::try_clone_to_owned and accompanying documentation #97178

Merged
merged 4 commits into from
Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions library/std/src/os/fd/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,20 @@ impl BorrowedFd<'_> {
}

impl OwnedFd {
/// Creates a new `OwnedFd` instance that shares the same underlying file handle
/// as the existing `OwnedFd` instance.
#[cfg(not(target_arch = "wasm32"))]
/// Creates a new `OwnedFd` instance that shares the same underlying file
/// description as the existing `OwnedFd` instance.
#[stable(feature = "io_safety", since = "1.63.0")]
pub fn try_clone(&self) -> crate::io::Result<Self> {
self.as_fd().try_clone_to_owned()
}
}

impl BorrowedFd<'_> {
/// Creates a new `OwnedFd` instance that shares the same underlying file
/// description as the existing `BorrowedFd` instance.
#[cfg(not(target_arch = "wasm32"))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does WASI not have a dup function?

Copy link
Member Author

@sunfishcode sunfishcode Jun 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not, today. Because of all the descriptor vs. description issues, the original WASI left dup out to give itself flexibility to figure out what it wanted to say. Today we now have a clear sense of how we want this to work, and I expect WASI will add a dup. When it does, we can remove this code from Rust.

#[stable(feature = "io_safety", since = "1.63.0")]
pub fn try_clone_to_owned(&self) -> crate::io::Result<OwnedFd> {
// We want to atomically duplicate this file descriptor and set the
// CLOEXEC flag, and currently that's done via F_DUPFD_CLOEXEC. This
// is a POSIX flag that was added to Linux in 2.6.24.
Expand All @@ -96,12 +105,14 @@ impl OwnedFd {
let cmd = libc::F_DUPFD;

let fd = cvt(unsafe { libc::fcntl(self.as_raw_fd(), cmd, 0) })?;
Ok(unsafe { Self::from_raw_fd(fd) })
Ok(unsafe { OwnedFd::from_raw_fd(fd) })
}

/// Creates a new `OwnedFd` instance that shares the same underlying file
/// description as the existing `BorrowedFd` instance.
#[cfg(target_arch = "wasm32")]
#[stable(feature = "io_safety", since = "1.63.0")]
pub fn try_clone(&self) -> crate::io::Result<Self> {
pub fn try_clone_to_owned(&self) -> crate::io::Result<OwnedFd> {
Err(crate::io::const_io_error!(
crate::io::ErrorKind::Unsupported,
"operation not supported on WASI yet",
Expand Down
28 changes: 19 additions & 9 deletions library/std/src/os/unix/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,30 @@
//! that they don't outlive the resource they point to. These are safe to
//! use. `BorrowedFd` values may be used in APIs which provide safe access to
//! any system call except for:
//!
//! - `close`, because that would end the dynamic lifetime of the resource
//! without ending the lifetime of the file descriptor.
//!
//! - `dup2`/`dup3`, in the second argument, because this argument is
//! closed and assigned a new resource, which may break the assumptions
//! other code using that file descriptor.
//! This list doesn't include `mmap`, since `mmap` does do a proper borrow of
//! its file descriptor argument. That said, `mmap` is unsafe for other
//! reasons: it operates on raw pointers, and it can have undefined behavior if
//! the underlying storage is mutated. Mutations may come from other processes,
//! or from the same process if the API provides `BorrowedFd` access, since as
//! mentioned earlier, `BorrowedFd` values may be used in APIs which provide
//! safe access to any system call. Consequently, code using `mmap` and
//! presenting a safe API must take full responsibility for ensuring that safe
//! Rust code cannot evoke undefined behavior through it.
//!
//! `BorrowedFd` values may be used in APIs which provide safe access to `dup`
//! system calls, so types implementing `AsFd` or `From<OwnedFd>` should not
//! assume they always have exclusive access to the underlying file
//! description.
//!
//! `BorrowedFd` values may also be used with `mmap`, since `mmap` uses the
//! provided file descriptor in a manner similar to `dup` and does not require
//! the `BorrowedFd` passed to it to live for the lifetime of the resulting
//! mapping. That said, `mmap` is unsafe for other reasons: it operates on raw
//! pointers, and it can have undefined behavior if the underlying storage is
//! mutated. Mutations may come from other processes, or from the same process
//! if the API provides `BorrowedFd` access, since as mentioned earlier,
//! `BorrowedFd` values may be used in APIs which provide safe access to any
//! system call. Consequently, code using `mmap` and presenting a safe API must
//! take full responsibility for ensuring that safe Rust code cannot evoke
//! undefined behavior through it.
//!
//! Like boxes, `OwnedFd` values conceptually own the resource they point to,
//! and free (close) it when they are dropped.
Expand Down
19 changes: 14 additions & 5 deletions library/std/src/os/windows/io/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,19 @@ impl TryFrom<HandleOrNull> for OwnedHandle {
}

impl OwnedHandle {
/// Creates a new `OwnedHandle` instance that shares the same underlying file handle
/// as the existing `OwnedHandle` instance.
/// Creates a new `OwnedHandle` instance that shares the same underlying
/// object as the existing `OwnedHandle` instance.
#[stable(feature = "io_safety", since = "1.63.0")]
pub fn try_clone(&self) -> crate::io::Result<Self> {
self.as_handle().try_clone_to_owned()
}
}

impl BorrowedHandle<'_> {
/// Creates a new `OwnedHandle` instance that shares the same underlying
/// object as the existing `BorrowedHandle` instance.
#[stable(feature = "io_safety", since = "1.63.0")]
pub fn try_clone_to_owned(&self) -> crate::io::Result<OwnedHandle> {
self.duplicate(0, false, c::DUPLICATE_SAME_ACCESS)
}

Expand All @@ -189,15 +198,15 @@ impl OwnedHandle {
access: c::DWORD,
inherit: bool,
options: c::DWORD,
) -> io::Result<Self> {
) -> io::Result<OwnedHandle> {
let handle = self.as_raw_handle();

// `Stdin`, `Stdout`, and `Stderr` can all hold null handles, such as
// in a process with a detached console. `DuplicateHandle` would fail
// if we passed it a null handle, but we can treat null as a valid
// handle which doesn't do any I/O, and allow it to be duplicated.
if handle.is_null() {
return unsafe { Ok(Self::from_raw_handle(handle)) };
return unsafe { Ok(OwnedHandle::from_raw_handle(handle)) };
}

let mut ret = ptr::null_mut();
Expand All @@ -213,7 +222,7 @@ impl OwnedHandle {
options,
)
})?;
unsafe { Ok(Self::from_raw_handle(ret)) }
unsafe { Ok(OwnedHandle::from_raw_handle(ret)) }
}
}

Expand Down
6 changes: 6 additions & 0 deletions library/std/src/os/windows/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@
//! dynamic lifetime of the resource without ending the lifetime of the
//! handle or socket.
//!
//! `BorrowedHandle` and `BorrowedSocket` values may be used in APIs which
//! provide safe access to `DuplicateHandle` and `WSADuplicateSocketW` and
//! related functions, so types implementing `AsHandle`, `AsSocket`,
//! `From<OwnedHandle>`, or `From<OwnedSocket>` should not assume they always
//! have exclusive access to the underlying object.
//!
//! Like boxes, `OwnedHandle` and `OwnedSocket` values conceptually own the
//! resource they point to, and free (close) it when they are dropped.
//!
Expand Down
41 changes: 25 additions & 16 deletions library/std/src/os/windows/io/socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,33 @@ impl BorrowedSocket<'_> {
}

impl OwnedSocket {
/// Creates a new `OwnedSocket` instance that shares the same underlying socket
/// as the existing `OwnedSocket` instance.
/// Creates a new `OwnedSocket` instance that shares the same underlying
/// object as the existing `OwnedSocket` instance.
#[stable(feature = "io_safety", since = "1.63.0")]
pub fn try_clone(&self) -> io::Result<Self> {
self.as_socket().try_clone_to_owned()
}

// FIXME(strict_provenance_magic): we defined RawSocket to be a u64 ;-;
#[cfg(not(target_vendor = "uwp"))]
pub(crate) fn set_no_inherit(&self) -> io::Result<()> {
cvt(unsafe {
c::SetHandleInformation(self.as_raw_socket() as c::HANDLE, c::HANDLE_FLAG_INHERIT, 0)
})
.map(drop)
}

#[cfg(target_vendor = "uwp")]
pub(crate) fn set_no_inherit(&self) -> io::Result<()> {
Err(io::const_io_error!(io::ErrorKind::Unsupported, "Unavailable on UWP"))
}
}

impl BorrowedSocket<'_> {
/// Creates a new `OwnedSocket` instance that shares the same underlying
/// object as the existing `BorrowedSocket` instance.
#[stable(feature = "io_safety", since = "1.63.0")]
pub fn try_clone_to_owned(&self) -> io::Result<OwnedSocket> {
let mut info = unsafe { mem::zeroed::<c::WSAPROTOCOL_INFO>() };
let result = unsafe {
c::WSADuplicateSocketW(self.as_raw_socket(), c::GetCurrentProcessId(), &mut info)
Expand Down Expand Up @@ -133,20 +156,6 @@ impl OwnedSocket {
}
}
}

// FIXME(strict_provenance_magic): we defined RawSocket to be a u64 ;-;
#[cfg(not(target_vendor = "uwp"))]
pub(crate) fn set_no_inherit(&self) -> io::Result<()> {
cvt(unsafe {
c::SetHandleInformation(self.as_raw_socket() as c::HANDLE, c::HANDLE_FLAG_INHERIT, 0)
})
.map(drop)
}

#[cfg(target_vendor = "uwp")]
pub(crate) fn set_no_inherit(&self) -> io::Result<()> {
Err(io::const_io_error!(io::ErrorKind::Unsupported, "Unavailable on UWP"))
}
}

/// Returns the last error from the Windows socket interface.
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/windows/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ impl Handle {
inherit: bool,
options: c::DWORD,
) -> io::Result<Self> {
Ok(Self(self.0.duplicate(access, inherit, options)?))
Ok(Self(self.0.as_handle().duplicate(access, inherit, options)?))
}

/// Performs a synchronous read.
Expand Down