From 7cab0d18b2ad00e7f3496546298fc759380a176b 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 | 5 ++++- README.md | 4 ++-- src/lib.rs | 1 + src/sys/mod.rs | 4 ++-- src/sys/unix/mod.rs | 2 -- src/sys/unix/read_guard.rs | 17 ++++++++--------- src/sys/unix/rw_lock.rs | 29 ++++++++++++++--------------- src/sys/unix/utils.rs | 9 --------- src/sys/unix/write_guard.rs | 19 +++++++++---------- src/sys/unsupported/read_guard.rs | 2 -- src/sys/unsupported/rw_lock.rs | 2 -- src/sys/unsupported/write_guard.rs | 2 -- src/sys/windows/read_guard.rs | 9 +++++---- src/sys/windows/rw_lock.rs | 13 +++++++------ src/sys/windows/write_guard.rs | 11 ++++++----- 15 files changed, 58 insertions(+), 71 deletions(-) delete mode 100644 src/sys/unix/utils.rs diff --git a/Cargo.toml b/Cargo.toml index 0794c85..164cf08 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,10 +12,13 @@ readme = "README.md" edition = "2018" [dependencies] -libc = "0.2.58" cfg-if = "1.0.0" +[target.'cfg(unix)'.dependencies] +rustix = "0.26.2" + [target.'cfg(windows)'.dependencies] +io-lifetimes = { version = "0.3.0", default-features = false } winapi = "0.3.7" [dev-dependencies] diff --git a/README.md b/README.md index db00f1d..3a9b856 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ $ cargo add fd-lock ``` ## Safety -This crate uses `unsafe` to interface with `libc` and `winapi`. All invariants +This crate uses `unsafe` on Windows to interface with `winapi`. All invariants have been carefully checked, and are manually enforced. ## Contributing @@ -51,7 +51,7 @@ look at some of these issues: ## References - [LockFile function - WDC](https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-lockfile) - [flock(2) - Linux Man Page](https://linux.die.net/man/2/flock) -- [`libc::flock`](https://docs.rs/libc/0.2.58/libc/fn.flock.html) +- [`rustix::fs::flock`](https://docs.rs/rustix/*/rustix/fs/fn.flock.html) - [`winapi::um::fileapi::LockFile`](https://docs.rs/winapi/0.3.7/x86_64-pc-windows-msvc/winapi/um/fileapi/fn.LockFile.html) ## License diff --git a/src/lib.rs b/src/lib.rs index 1e941d0..5ea90b5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,6 +31,7 @@ #![forbid(future_incompatible)] #![deny(missing_debug_implementations, nonstandard_style)] #![warn(missing_docs, missing_doc_code_examples)] +#![cfg_attr(io_lifetimes_use_std, feature(io_safety))] mod read_guard; mod rw_lock; diff --git a/src/sys/mod.rs b/src/sys/mod.rs index 3c6391e..d5257af 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::io_lifetimes::AsFd as AsRaw; } 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 AsRaw; } else { mod unsupported; pub use unsupported; diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs index 9d7bd2c..023e691 100644 --- a/src/sys/unix/mod.rs +++ b/src/sys/unix/mod.rs @@ -2,8 +2,6 @@ mod read_guard; mod rw_lock; mod write_guard; -pub(crate) mod utils; - pub use read_guard::RwLockReadGuard; pub use rw_lock::RwLock; pub use write_guard::RwLockWriteGuard; diff --git a/src/sys/unix/read_guard.rs b/src/sys/unix/read_guard.rs index bf7490b..0462515 100644 --- a/src/sys/unix/read_guard.rs +++ b/src/sys/unix/read_guard.rs @@ -1,22 +1,21 @@ -use libc::{flock, LOCK_UN}; +use rustix::fs::{flock, FlockOperation}; +use rustix::io_lifetimes::AsFd; use std::ops; -use std::os::unix::io::AsRawFd; -use super::utils::syscall; 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] @@ -25,10 +24,10 @@ impl ops::Deref for RwLockReadGuard<'_, T> { } } -impl Drop for RwLockReadGuard<'_, T> { +impl Drop for RwLockReadGuard<'_, T> { #[inline] fn drop(&mut self) { - let fd = self.lock.inner.as_raw_fd(); - let _ = syscall(unsafe { flock(fd, LOCK_UN) }); + let fd = self.lock.inner.as_fd(); + let _ = flock(&fd, FlockOperation::Unlock).ok(); } } diff --git a/src/sys/unix/rw_lock.rs b/src/sys/unix/rw_lock.rs index 70c9860..53b1084 100644 --- a/src/sys/unix/rw_lock.rs +++ b/src/sys/unix/rw_lock.rs @@ -1,16 +1,15 @@ -use libc::{flock, LOCK_EX, LOCK_NB, LOCK_SH}; +use rustix::fs::{flock, FlockOperation}; +use rustix::io_lifetimes::AsFd; use std::io::{self, Error, ErrorKind}; -use std::os::unix::io::AsRawFd; -use super::utils::syscall; 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,34 +17,34 @@ impl RwLock { #[inline] pub fn write(&mut self) -> io::Result> { - let fd = self.inner.as_raw_fd(); - syscall(unsafe { flock(fd, LOCK_EX) })?; + let fd = self.inner.as_fd(); + flock(&fd, FlockOperation::LockExclusive)?; Ok(RwLockWriteGuard::new(self)) } #[inline] pub fn try_write(&mut self) -> Result, Error> { - let fd = self.inner.as_raw_fd(); - syscall(unsafe { flock(fd, LOCK_EX | LOCK_NB) }).map_err(|err| match err.kind() { + let fd = self.inner.as_fd(); + flock(&fd, FlockOperation::NonBlockingLockExclusive).map_err(|err| match err.kind() { ErrorKind::AlreadyExists => ErrorKind::WouldBlock.into(), - _ => err, + _ => Error::from(err), })?; Ok(RwLockWriteGuard::new(self)) } #[inline] pub fn read(&self) -> io::Result> { - let fd = self.inner.as_raw_fd(); - syscall(unsafe { flock(fd, LOCK_SH) })?; + let fd = self.inner.as_fd(); + flock(&fd, FlockOperation::LockShared)?; Ok(RwLockReadGuard::new(self)) } #[inline] pub fn try_read(&self) -> Result, Error> { - let fd = self.inner.as_raw_fd(); - syscall(unsafe { flock(fd, LOCK_SH | LOCK_NB) }).map_err(|err| match err.kind() { + let fd = self.inner.as_fd(); + flock(&fd, FlockOperation::NonBlockingLockShared).map_err(|err| match err.kind() { ErrorKind::AlreadyExists => ErrorKind::WouldBlock.into(), - _ => err, + _ => Error::from(err), })?; Ok(RwLockReadGuard::new(self)) } diff --git a/src/sys/unix/utils.rs b/src/sys/unix/utils.rs deleted file mode 100644 index 191dd0c..0000000 --- a/src/sys/unix/utils.rs +++ /dev/null @@ -1,9 +0,0 @@ -use std::io; -use std::os::raw::c_int; - -pub(crate) fn syscall(int: c_int) -> io::Result<()> { - match int { - 0 => Ok(()), - _ => Err(io::Error::last_os_error()), - } -} diff --git a/src/sys/unix/write_guard.rs b/src/sys/unix/write_guard.rs index 6060d60..ba061fc 100644 --- a/src/sys/unix/write_guard.rs +++ b/src/sys/unix/write_guard.rs @@ -1,22 +1,21 @@ -use libc::{flock, LOCK_UN}; +use rustix::fs::{flock, FlockOperation}; +use rustix::io_lifetimes::AsFd; use std::ops; -use std::os::unix::io::AsRawFd; -use super::utils::syscall; 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] @@ -25,17 +24,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 fd = self.lock.inner.as_raw_fd(); - let _ = syscall(unsafe { flock(fd, LOCK_UN) }); + let fd = self.lock.inner.as_fd(); + let _ = flock(&fd, FlockOperation::Unlock).ok(); } } diff --git a/src/sys/unsupported/read_guard.rs b/src/sys/unsupported/read_guard.rs index c0ea175..7d6b4c7 100644 --- a/src/sys/unsupported/read_guard.rs +++ b/src/sys/unsupported/read_guard.rs @@ -1,8 +1,6 @@ -use libc::{flock, LOCK_UN}; use std::ops; use std::os::unix::io::AsRawFd; -use super::utils::syscall; use super::RwLock; #[derive(Debug)] diff --git a/src/sys/unsupported/rw_lock.rs b/src/sys/unsupported/rw_lock.rs index 856e5d7..fb11da8 100644 --- a/src/sys/unsupported/rw_lock.rs +++ b/src/sys/unsupported/rw_lock.rs @@ -1,8 +1,6 @@ -use libc::{flock, LOCK_EX, LOCK_NB, LOCK_SH}; use std::io::{self, Error, ErrorKind}; use std::os::unix::io::AsRawFd; -use super::utils::syscall; use super::{RwLockReadGuard, RwLockWriteGuard}; #[derive(Debug)] diff --git a/src/sys/unsupported/write_guard.rs b/src/sys/unsupported/write_guard.rs index 28a6a64..19727c3 100644 --- a/src/sys/unsupported/write_guard.rs +++ b/src/sys/unsupported/write_guard.rs @@ -1,8 +1,6 @@ -use libc::{flock, LOCK_UN}; use std::ops; use std::os::unix::io::AsRawFd; -use super::utils::syscall; use super::RwLock; #[derive(Debug)] diff --git a/src/sys/windows/read_guard.rs b/src/sys/windows/read_guard.rs index 768f809..e4a89a4 100644 --- a/src/sys/windows/read_guard.rs +++ b/src/sys/windows/read_guard.rs @@ -1,3 +1,4 @@ +use io_lifetimes::AsHandle; use winapi::um::fileapi::UnlockFile; use std::ops; @@ -7,11 +8,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] @@ -20,10 +21,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(); + let handle = self.lock.inner.as_handle().as_raw_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 9baf449..85a2188 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; @@ -8,11 +9,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 } @@ -21,7 +22,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(); + let handle = self.inner.as_handle().as_raw_handle(); let overlapped = Overlapped::zero(); let flags = 0; syscall(unsafe { LockFileEx(handle, flags, 0, 1, 0, overlapped.raw()) })?; @@ -30,7 +31,7 @@ impl RwLock { #[inline] pub fn try_read(&self) -> io::Result> { - let handle = self.inner.as_raw_handle(); + let handle = self.inner.as_handle().as_raw_handle(); let overlapped = Overlapped::zero(); let flags = LOCKFILE_FAIL_IMMEDIATELY; @@ -42,7 +43,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(); + let handle = self.inner.as_handle().as_raw_handle(); let overlapped = Overlapped::zero(); let flags = LOCKFILE_EXCLUSIVE_LOCK; syscall(unsafe { LockFileEx(handle, flags, 0, 1, 0, overlapped.raw()) })?; @@ -51,7 +52,7 @@ impl RwLock { #[inline] pub fn try_write(&mut self) -> io::Result> { - let handle = self.inner.as_raw_handle(); + let handle = self.inner.as_handle().as_raw_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 a299547..53b42f3 100644 --- a/src/sys/windows/write_guard.rs +++ b/src/sys/windows/write_guard.rs @@ -1,3 +1,4 @@ +use io_lifetimes::AsHandle; use winapi::um::fileapi::UnlockFile; use std::ops; @@ -7,11 +8,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] @@ -20,17 +21,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(); + let handle = self.lock.inner.as_handle().as_raw_handle(); syscall(unsafe { UnlockFile(handle, 0, 0, 1, 0) }) .expect("Could not unlock the file descriptor"); }