Skip to content

Commit

Permalink
Use rustix instead of calling libc directly.
Browse files Browse the repository at this point in the history
Use rustix instead of calling libc directly, which factors out several
unsafe blocks and simplifies error handling.

There is still one `unsafe` block needed, to dereference raw file
descriptors passed in from the user, since the crate still uses
`AsRawFd`. Once I/O safety is stablized in std, this crate can switch
to using `AsFd`, which will eliminate the last `unsafe` block.

This splits out just the API-compatible parts of yoshuawuyts#14. It does not include
the changes to use the `AsFd` trait in the public API, so it's not an
API-breaking change.
  • Loading branch information
sunfishcode committed Jan 14, 2022
1 parent 95d2227 commit 96badb5
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 42 deletions.
4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ readme = "README.md"
edition = "2018"

[dependencies]
libc = "0.2.58"
cfg-if = "1.0.0"

[target.'cfg(windows)'.dependencies.windows-sys]
Expand All @@ -23,5 +22,8 @@ features = [
"Win32_System_IO",
]

[target.'cfg(unix)'.dependencies]
rustix = "0.31.3"

[dev-dependencies]
tempfile = "3.0.8"
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -50,8 +50,8 @@ 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)
- [flock(2) - Linux Man Page](https://man7.org/linux/man-pages/man2/flock.2.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
Expand Down
2 changes: 0 additions & 2 deletions src/sys/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
6 changes: 2 additions & 4 deletions src/sys/unix/read_guard.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use libc::{flock, LOCK_UN};
use rustix::fs::{flock, FlockOperation};
use std::ops;
use std::os::unix::io::AsRawFd;

use super::utils::syscall;
use super::RwLock;

#[derive(Debug)]
Expand All @@ -28,7 +27,6 @@ impl<T: AsRawFd> ops::Deref for RwLockReadGuard<'_, T> {
impl<T: AsRawFd> 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 _ = flock(&self.lock.as_fd(), FlockOperation::Unlock).ok();
}
}
36 changes: 23 additions & 13 deletions src/sys/unix/rw_lock.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use libc::{flock, LOCK_EX, LOCK_NB, LOCK_SH};
use rustix::fd::BorrowedFd;
use rustix::fs::{flock, FlockOperation};
use std::io::{self, Error, ErrorKind};
use std::os::unix::io::AsRawFd;

use super::utils::syscall;
use super::{RwLockReadGuard, RwLockWriteGuard};

#[derive(Debug)]
Expand All @@ -18,34 +18,34 @@ impl<T: AsRawFd> RwLock<T> {

#[inline]
pub fn write(&mut self) -> io::Result<RwLockWriteGuard<'_, T>> {
let fd = self.inner.as_raw_fd();
syscall(unsafe { flock(fd, LOCK_EX) })?;
flock(&self.as_fd(), FlockOperation::LockExclusive)?;
Ok(RwLockWriteGuard::new(self))
}

#[inline]
pub fn try_write(&mut self) -> Result<RwLockWriteGuard<'_, T>, Error> {
let fd = self.inner.as_raw_fd();
syscall(unsafe { flock(fd, LOCK_EX | LOCK_NB) }).map_err(|err| match err.kind() {
flock(&self.as_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<RwLockReadGuard<'_, T>> {
let fd = self.inner.as_raw_fd();
syscall(unsafe { flock(fd, LOCK_SH) })?;
flock(&self.as_fd(), FlockOperation::LockShared)?;
Ok(RwLockReadGuard::new(self))
}

#[inline]
pub fn try_read(&self) -> Result<RwLockReadGuard<'_, T>, Error> {
let fd = self.inner.as_raw_fd();
syscall(unsafe { flock(fd, LOCK_SH | LOCK_NB) }).map_err(|err| match err.kind() {
ErrorKind::AlreadyExists => ErrorKind::WouldBlock.into(),
_ => err,
flock(&self.as_fd(), FlockOperation::NonBlockingLockShared).map_err(|err| {
match err.kind() {
ErrorKind::AlreadyExists => ErrorKind::WouldBlock.into(),
_ => Error::from(err),
}
})?;
Ok(RwLockReadGuard::new(self))
}
Expand All @@ -57,4 +57,14 @@ impl<T: AsRawFd> RwLock<T> {
{
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_fd(self.inner.as_raw_fd()) }
}
}
9 changes: 0 additions & 9 deletions src/sys/unix/utils.rs

This file was deleted.

6 changes: 2 additions & 4 deletions src/sys/unix/write_guard.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use libc::{flock, LOCK_UN};
use rustix::fs::{flock, FlockOperation};
use std::ops;
use std::os::unix::io::AsRawFd;

use super::utils::syscall;
use super::RwLock;

#[derive(Debug)]
Expand Down Expand Up @@ -35,7 +34,6 @@ impl<T: AsRawFd> ops::DerefMut for RwLockWriteGuard<'_, T> {
impl<T: AsRawFd> 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 _ = flock(&self.lock.as_fd(), FlockOperation::Unlock).ok();
}
}
2 changes: 0 additions & 2 deletions src/sys/unsupported/read_guard.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
2 changes: 0 additions & 2 deletions src/sys/unsupported/rw_lock.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
2 changes: 0 additions & 2 deletions src/sys/unsupported/write_guard.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down

0 comments on commit 96badb5

Please sign in to comment.