From 39e84e064be4de58c8d01c213ac677aeda88d2c2 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 17 Aug 2021 15:28:20 -0700 Subject: [PATCH] Use the new I/O safety traits, using rustix Switch public APIs from `AsRawFd`/`AsRawHandle` to the new I/O safety traits `AsFd`/`AsHandle`. On Unix platforms, use rustix to avoid manipulating raw file descriptors altogether. --- Cargo.toml | 3 +++ src/lib.rs | 1 + src/read_guard.rs | 8 ++++---- src/rw_lock.rs | 4 ++-- src/sys/mod.rs | 4 ++-- src/sys/unix/read_guard.rs | 12 +++++------ src/sys/unix/rw_lock.rs | 37 +++++++++++++--------------------- src/sys/unix/write_guard.rs | 14 ++++++------- src/sys/windows/read_guard.rs | 9 +++++---- src/sys/windows/rw_lock.rs | 13 ++++++------ src/sys/windows/write_guard.rs | 11 +++++----- src/write_guard.rs | 10 ++++----- 12 files changed, 62 insertions(+), 64 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9f6c15c..4473c30 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,5 +25,8 @@ features = [ [target.'cfg(unix)'.dependencies] rustix = { version = "0.35.6", features = ["fs"] } +[target.'cfg(windows)'.dependencies] +io-lifetimes = "0.6.0" + [dev-dependencies] tempfile = "3.0.8" diff --git a/src/lib.rs b/src/lib.rs index 9d3d619..6547cb0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,6 +31,7 @@ #![forbid(future_incompatible)] #![deny(missing_debug_implementations, nonstandard_style)] #![cfg_attr(doc, warn(missing_docs, rustdoc::missing_doc_code_examples))] +#![cfg_attr(io_lifetimes_use_std, feature(io_safety))] mod read_guard; mod rw_lock; diff --git a/src/read_guard.rs b/src/read_guard.rs index 29ed06d..c29c21c 100644 --- a/src/read_guard.rs +++ b/src/read_guard.rs @@ -17,17 +17,17 @@ use crate::sys; /// Dropping this type may panic if the lock fails to unlock. #[must_use = "if unused the RwLock will immediately unlock"] #[derive(Debug)] -pub struct RwLockReadGuard<'lock, T: sys::AsRaw> { +pub struct RwLockReadGuard<'lock, T: sys::AsOpenFile> { guard: sys::RwLockReadGuard<'lock, T>, } -impl<'lock, T: sys::AsRaw> RwLockReadGuard<'lock, T> { +impl<'lock, T: sys::AsOpenFile> RwLockReadGuard<'lock, T> { pub(crate) fn new(guard: sys::RwLockReadGuard<'lock, T>) -> Self { Self { guard } } } -impl ops::Deref for RwLockReadGuard<'_, T> { +impl ops::Deref for RwLockReadGuard<'_, T> { type Target = T; #[inline] @@ -37,7 +37,7 @@ impl ops::Deref for RwLockReadGuard<'_, T> { } /// Release the lock. -impl Drop for RwLockReadGuard<'_, T> { +impl Drop for RwLockReadGuard<'_, T> { #[inline] fn drop(&mut self) {} } diff --git a/src/rw_lock.rs b/src/rw_lock.rs index 77f9d22..872dddd 100644 --- a/src/rw_lock.rs +++ b/src/rw_lock.rs @@ -10,11 +10,11 @@ use std::io; /// underlying data (exclusive access) and the read portion of this lock typically /// allows for read-only access (shared access). #[derive(Debug)] -pub struct RwLock { +pub struct RwLock { lock: sys::RwLock, } -impl RwLock { +impl RwLock { /// Create a new instance. /// /// # Examples diff --git a/src/sys/mod.rs b/src/sys/mod.rs index 3c6391e..e06c6e0 100644 --- a/src/sys/mod.rs +++ b/src/sys/mod.rs @@ -4,12 +4,12 @@ cfg_if! { if #[cfg(unix)] { mod unix; pub use unix::*; - pub(crate) use std::os::unix::prelude::AsRawFd as AsRaw; + pub(crate) use rustix::fd::AsFd as AsOpenFile; } else if #[cfg(windows)] { mod windows; pub use windows::*; #[doc(no_inline)] - pub(crate) use std::os::windows::prelude::AsRawHandle as AsRaw; + pub(crate) use io_lifetimes::AsHandle as AsOpenFile; } else { mod unsupported; pub use unsupported; diff --git a/src/sys/unix/read_guard.rs b/src/sys/unix/read_guard.rs index 6d67187..972bb56 100644 --- a/src/sys/unix/read_guard.rs +++ b/src/sys/unix/read_guard.rs @@ -1,21 +1,21 @@ +use rustix::fd::AsFd; use rustix::fs::{flock, FlockOperation}; use std::ops; -use std::os::unix::io::AsRawFd; use super::RwLock; #[derive(Debug)] -pub struct RwLockReadGuard<'lock, T: AsRawFd> { +pub struct RwLockReadGuard<'lock, T: AsFd> { lock: &'lock RwLock, } -impl<'lock, T: AsRawFd> RwLockReadGuard<'lock, T> { +impl<'lock, T: AsFd> RwLockReadGuard<'lock, T> { pub(crate) fn new(lock: &'lock RwLock) -> Self { Self { lock } } } -impl ops::Deref for RwLockReadGuard<'_, T> { +impl ops::Deref for RwLockReadGuard<'_, T> { type Target = T; #[inline] @@ -24,9 +24,9 @@ impl ops::Deref for RwLockReadGuard<'_, T> { } } -impl Drop for RwLockReadGuard<'_, T> { +impl Drop for RwLockReadGuard<'_, T> { #[inline] fn drop(&mut self) { - let _ = flock(&self.lock.as_fd(), FlockOperation::Unlock).ok(); + let _ = flock(&self.lock.inner.as_fd(), FlockOperation::Unlock).ok(); } } diff --git a/src/sys/unix/rw_lock.rs b/src/sys/unix/rw_lock.rs index c33ce5c..992925e 100644 --- a/src/sys/unix/rw_lock.rs +++ b/src/sys/unix/rw_lock.rs @@ -1,16 +1,15 @@ -use rustix::fd::BorrowedFd; +use rustix::fd::AsFd; use rustix::fs::{flock, FlockOperation}; use std::io::{self, Error, ErrorKind}; -use std::os::unix::io::AsRawFd; use super::{RwLockReadGuard, RwLockWriteGuard}; #[derive(Debug)] -pub struct RwLock { +pub struct RwLock { pub(crate) inner: T, } -impl RwLock { +impl RwLock { #[inline] pub fn new(inner: T) -> Self { RwLock { inner } @@ -18,15 +17,17 @@ impl RwLock { #[inline] pub fn write(&mut self) -> io::Result> { - flock(&self.as_fd(), FlockOperation::LockExclusive)?; + flock(&self.inner.as_fd(), FlockOperation::LockExclusive)?; Ok(RwLockWriteGuard::new(self)) } #[inline] pub fn try_write(&mut self) -> Result, Error> { - flock(&self.as_fd(), FlockOperation::NonBlockingLockExclusive).map_err(|err| match err - .kind() - { + flock( + &self.inner.as_fd(), + FlockOperation::NonBlockingLockExclusive, + ) + .map_err(|err| match err.kind() { ErrorKind::AlreadyExists => ErrorKind::WouldBlock.into(), _ => Error::from(err), })?; @@ -35,18 +36,18 @@ impl RwLock { #[inline] pub fn read(&self) -> io::Result> { - flock(&self.as_fd(), FlockOperation::LockShared)?; + flock(&self.inner.as_fd(), FlockOperation::LockShared)?; Ok(RwLockReadGuard::new(self)) } #[inline] pub fn try_read(&self) -> Result, Error> { - flock(&self.as_fd(), FlockOperation::NonBlockingLockShared).map_err(|err| { - match err.kind() { + flock(&self.inner.as_fd(), FlockOperation::NonBlockingLockShared).map_err( + |err| match err.kind() { ErrorKind::AlreadyExists => ErrorKind::WouldBlock.into(), _ => Error::from(err), - } - })?; + }, + )?; Ok(RwLockReadGuard::new(self)) } @@ -57,14 +58,4 @@ impl RwLock { { self.inner } - - #[inline] - pub(crate) fn as_fd(&self) -> BorrowedFd<'_> { - // Safety: We assume that `self.inner`'s file descriptor is valid for - // at least the lifetime of `self`. - // - // Once I/O safety is stablized in std, we can switch the public API to - // use `AsFd` instead of `AsRawFd` and eliminate this `unsafe` block. - unsafe { BorrowedFd::borrow_raw(self.inner.as_raw_fd()) } - } } diff --git a/src/sys/unix/write_guard.rs b/src/sys/unix/write_guard.rs index a12bdf9..1bdc1a4 100644 --- a/src/sys/unix/write_guard.rs +++ b/src/sys/unix/write_guard.rs @@ -1,21 +1,21 @@ +use rustix::fd::AsFd; use rustix::fs::{flock, FlockOperation}; use std::ops; -use std::os::unix::io::AsRawFd; use super::RwLock; #[derive(Debug)] -pub struct RwLockWriteGuard<'lock, T: AsRawFd> { +pub struct RwLockWriteGuard<'lock, T: AsFd> { lock: &'lock mut RwLock, } -impl<'lock, T: AsRawFd> RwLockWriteGuard<'lock, T> { +impl<'lock, T: AsFd> RwLockWriteGuard<'lock, T> { pub(crate) fn new(lock: &'lock mut RwLock) -> Self { Self { lock } } } -impl ops::Deref for RwLockWriteGuard<'_, T> { +impl ops::Deref for RwLockWriteGuard<'_, T> { type Target = T; #[inline] @@ -24,16 +24,16 @@ impl ops::Deref for RwLockWriteGuard<'_, T> { } } -impl ops::DerefMut for RwLockWriteGuard<'_, T> { +impl ops::DerefMut for RwLockWriteGuard<'_, T> { #[inline] fn deref_mut(&mut self) -> &mut Self::Target { &mut self.lock.inner } } -impl Drop for RwLockWriteGuard<'_, T> { +impl Drop for RwLockWriteGuard<'_, T> { #[inline] fn drop(&mut self) { - let _ = flock(&self.lock.as_fd(), FlockOperation::Unlock).ok(); + let _ = flock(&self.lock.inner.as_fd(), FlockOperation::Unlock).ok(); } } diff --git a/src/sys/windows/read_guard.rs b/src/sys/windows/read_guard.rs index 28f09bb..31c3f90 100644 --- a/src/sys/windows/read_guard.rs +++ b/src/sys/windows/read_guard.rs @@ -1,3 +1,4 @@ +use io_lifetimes::AsHandle; use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::Storage::FileSystem::UnlockFile; @@ -8,11 +9,11 @@ use super::utils::syscall; use super::RwLock; #[derive(Debug)] -pub struct RwLockReadGuard<'lock, T: AsRawHandle> { +pub struct RwLockReadGuard<'lock, T: AsHandle> { pub(crate) lock: &'lock RwLock, } -impl ops::Deref for RwLockReadGuard<'_, T> { +impl ops::Deref for RwLockReadGuard<'_, T> { type Target = T; #[inline] @@ -21,10 +22,10 @@ impl ops::Deref for RwLockReadGuard<'_, T> { } } -impl Drop for RwLockReadGuard<'_, T> { +impl Drop for RwLockReadGuard<'_, T> { #[inline] fn drop(&mut self) { - let handle = self.lock.inner.as_raw_handle() as HANDLE; + let handle = self.lock.inner.as_handle().as_raw_handle() as HANDLE; syscall(unsafe { UnlockFile(handle, 0, 0, 1, 0) }) .expect("Could not unlock the file descriptor"); } diff --git a/src/sys/windows/rw_lock.rs b/src/sys/windows/rw_lock.rs index 778b791..30203eb 100644 --- a/src/sys/windows/rw_lock.rs +++ b/src/sys/windows/rw_lock.rs @@ -1,3 +1,4 @@ +use io_lifetimes::AsHandle; use std::io::{self, Error, ErrorKind}; use std::os::windows::io::AsRawHandle; @@ -10,11 +11,11 @@ use super::utils::{syscall, Overlapped}; use super::{RwLockReadGuard, RwLockWriteGuard}; #[derive(Debug)] -pub struct RwLock { +pub struct RwLock { pub(crate) inner: T, } -impl RwLock { +impl RwLock { #[inline] pub fn new(inner: T) -> Self { RwLock { inner } @@ -23,7 +24,7 @@ impl RwLock { #[inline] pub fn read(&self) -> io::Result> { // See: https://stackoverflow.com/a/9186532, https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-lockfileex - let handle = self.inner.as_raw_handle() as HANDLE; + let handle = self.inner.as_handle().as_raw_handle() as HANDLE; let overlapped = Overlapped::zero(); let flags = 0; syscall(unsafe { LockFileEx(handle, flags, 0, 1, 0, overlapped.raw()) })?; @@ -32,7 +33,7 @@ impl RwLock { #[inline] pub fn try_read(&self) -> io::Result> { - let handle = self.inner.as_raw_handle() as HANDLE; + let handle = self.inner.as_handle().as_raw_handle() as HANDLE; let overlapped = Overlapped::zero(); let flags = LOCKFILE_FAIL_IMMEDIATELY; @@ -44,7 +45,7 @@ impl RwLock { #[inline] pub fn write(&mut self) -> io::Result> { // See: https://stackoverflow.com/a/9186532, https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-lockfileex - let handle = self.inner.as_raw_handle() as HANDLE; + let handle = self.inner.as_handle().as_raw_handle() as HANDLE; let overlapped = Overlapped::zero(); let flags = LOCKFILE_EXCLUSIVE_LOCK; syscall(unsafe { LockFileEx(handle, flags, 0, 1, 0, overlapped.raw()) })?; @@ -53,7 +54,7 @@ impl RwLock { #[inline] pub fn try_write(&mut self) -> io::Result> { - let handle = self.inner.as_raw_handle() as HANDLE; + let handle = self.inner.as_handle().as_raw_handle() as HANDLE; let overlapped = Overlapped::zero(); let flags = LOCKFILE_FAIL_IMMEDIATELY | LOCKFILE_EXCLUSIVE_LOCK; diff --git a/src/sys/windows/write_guard.rs b/src/sys/windows/write_guard.rs index 5494bd9..2954b77 100644 --- a/src/sys/windows/write_guard.rs +++ b/src/sys/windows/write_guard.rs @@ -1,3 +1,4 @@ +use io_lifetimes::AsHandle; use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::Storage::FileSystem::UnlockFile; @@ -8,11 +9,11 @@ use super::utils::syscall; use super::RwLock; #[derive(Debug)] -pub struct RwLockWriteGuard<'lock, T: AsRawHandle> { +pub struct RwLockWriteGuard<'lock, T: AsHandle> { pub(crate) lock: &'lock mut RwLock, } -impl ops::Deref for RwLockWriteGuard<'_, T> { +impl ops::Deref for RwLockWriteGuard<'_, T> { type Target = T; #[inline] @@ -21,17 +22,17 @@ impl ops::Deref for RwLockWriteGuard<'_, T> { } } -impl ops::DerefMut for RwLockWriteGuard<'_, T> { +impl ops::DerefMut for RwLockWriteGuard<'_, T> { #[inline] fn deref_mut(&mut self) -> &mut Self::Target { &mut self.lock.inner } } -impl Drop for RwLockWriteGuard<'_, T> { +impl Drop for RwLockWriteGuard<'_, T> { #[inline] fn drop(&mut self) { - let handle = self.lock.inner.as_raw_handle() as HANDLE; + let handle = self.lock.inner.as_handle().as_raw_handle() as HANDLE; syscall(unsafe { UnlockFile(handle, 0, 0, 1, 0) }) .expect("Could not unlock the file descriptor"); } diff --git a/src/write_guard.rs b/src/write_guard.rs index e218e38..9480b3a 100644 --- a/src/write_guard.rs +++ b/src/write_guard.rs @@ -17,17 +17,17 @@ use crate::sys; /// Dropping this type may panic if the lock fails to unlock. #[must_use = "if unused the RwLock will immediately unlock"] #[derive(Debug)] -pub struct RwLockWriteGuard<'lock, T: sys::AsRaw> { +pub struct RwLockWriteGuard<'lock, T: sys::AsOpenFile> { guard: sys::RwLockWriteGuard<'lock, T>, } -impl<'lock, T: sys::AsRaw> RwLockWriteGuard<'lock, T> { +impl<'lock, T: sys::AsOpenFile> RwLockWriteGuard<'lock, T> { pub(crate) fn new(guard: sys::RwLockWriteGuard<'lock, T>) -> Self { Self { guard } } } -impl ops::Deref for RwLockWriteGuard<'_, T> { +impl ops::Deref for RwLockWriteGuard<'_, T> { type Target = T; #[inline] @@ -36,7 +36,7 @@ impl ops::Deref for RwLockWriteGuard<'_, T> { } } -impl ops::DerefMut for RwLockWriteGuard<'_, T> { +impl ops::DerefMut for RwLockWriteGuard<'_, T> { #[inline] fn deref_mut(&mut self) -> &mut Self::Target { self.guard.deref_mut() @@ -44,7 +44,7 @@ impl ops::DerefMut for RwLockWriteGuard<'_, T> { } /// Release the lock. -impl Drop for RwLockWriteGuard<'_, T> { +impl Drop for RwLockWriteGuard<'_, T> { #[inline] fn drop(&mut self) {} }