From eb37bbcebc3f6d0981eef892817f3a4570e35907 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 19 May 2022 06:41:35 -0700 Subject: [PATCH] Document that `BorrowedFd` may be used to do a `dup`. --- library/std/src/os/unix/io/mod.rs | 28 +++++++++++++++++-------- library/std/src/os/windows/io/handle.rs | 6 +++--- library/std/src/os/windows/io/mod.rs | 6 ++++++ library/std/src/sys/windows/handle.rs | 2 +- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/library/std/src/os/unix/io/mod.rs b/library/std/src/os/unix/io/mod.rs index e3a7cfc8d2a..7556d3ad0b2 100644 --- a/library/std/src/os/unix/io/mod.rs +++ b/library/std/src/os/unix/io/mod.rs @@ -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` 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. diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs index 2f7c07c7910..91b886c0888 100644 --- a/library/std/src/os/windows/io/handle.rs +++ b/library/std/src/os/windows/io/handle.rs @@ -189,7 +189,7 @@ pub(crate) fn duplicate( access: c::DWORD, inherit: bool, options: c::DWORD, - ) -> io::Result { + ) -> io::Result { let handle = self.as_raw_handle(); // `Stdin`, `Stdout`, and `Stderr` can all hold null handles, such as @@ -197,7 +197,7 @@ pub(crate) fn duplicate( // 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(); @@ -213,7 +213,7 @@ pub(crate) fn duplicate( options, ) })?; - unsafe { Ok(Self::from_raw_handle(ret)) } + unsafe { Ok(OwnedHandle::from_raw_handle(ret)) } } } diff --git a/library/std/src/os/windows/io/mod.rs b/library/std/src/os/windows/io/mod.rs index 31545059707..e2a401fb696 100644 --- a/library/std/src/os/windows/io/mod.rs +++ b/library/std/src/os/windows/io/mod.rs @@ -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`, or `From` 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. //! diff --git a/library/std/src/sys/windows/handle.rs b/library/std/src/sys/windows/handle.rs index ef9a8bd6900..1e7b6e1eab0 100644 --- a/library/std/src/sys/windows/handle.rs +++ b/library/std/src/sys/windows/handle.rs @@ -218,7 +218,7 @@ pub fn duplicate( inherit: bool, options: c::DWORD, ) -> io::Result { - Ok(Self(self.0.duplicate(access, inherit, options)?)) + Ok(Self(self.0.as_handle().duplicate(access, inherit, options)?)) } /// Performs a synchronous read. -- GitLab