Skip to content

Commit

Permalink
Replace FdGuard with OwnedFd
Browse files Browse the repository at this point in the history
An OwnedFd provides the same guarantees: that the underlying file
descriptor is closed only once. The file descriptor is behind an Arc, so
it will be closed once the last copy of the Arc is dropped.
  • Loading branch information
WhyNotHugo committed Jan 14, 2025
1 parent 42e4a83 commit ad90da8
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 173 deletions.
11 changes: 5 additions & 6 deletions src/events.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use std::{
ffi::{OsStr, OsString},
mem,
os::unix::ffi::OsStrExt,
os::{fd::OwnedFd, unix::ffi::OsStrExt},
sync::Weak,
};

use inotify_sys as ffi;

use crate::fd_guard::FdGuard;
use crate::watches::WatchDescriptor;

/// Iterator over inotify events
Expand All @@ -19,14 +18,14 @@ use crate::watches::WatchDescriptor;
/// [`Inotify::read_events`]: crate::Inotify::read_events
#[derive(Debug)]
pub struct Events<'a> {
fd: Weak<FdGuard>,
fd: Weak<OwnedFd>,
buffer: &'a [u8],
num_bytes: usize,
pos: usize,
}

impl<'a> Events<'a> {
pub(crate) fn new(fd: Weak<FdGuard>, buffer: &'a [u8], num_bytes: usize) -> Self {
pub(crate) fn new(fd: Weak<OwnedFd>, buffer: &'a [u8], num_bytes: usize) -> Self {
Events {
fd,
buffer,
Expand Down Expand Up @@ -97,7 +96,7 @@ pub struct Event<S> {
}

impl<'a> Event<&'a OsStr> {
fn new(fd: Weak<FdGuard>, event: &ffi::inotify_event, name: &'a OsStr) -> Self {
fn new(fd: Weak<OwnedFd>, event: &ffi::inotify_event, name: &'a OsStr) -> Self {
let mask = EventMask::from_bits(event.mask)
.expect("Failed to convert event mask. This indicates a bug.");

Expand All @@ -123,7 +122,7 @@ impl<'a> Event<&'a OsStr> {
/// # Panics
///
/// Panics if the buffer does not contain a full event, including its name.
pub(crate) fn from_buffer(fd: Weak<FdGuard>, buffer: &'a [u8]) -> (usize, Self) {
pub(crate) fn from_buffer(fd: Weak<OwnedFd>, buffer: &'a [u8]) -> (usize, Self) {
let event_size = mem::size_of::<ffi::inotify_event>();

// Make sure that the buffer is big enough to contain an event, without
Expand Down
83 changes: 0 additions & 83 deletions src/fd_guard.rs

This file was deleted.

69 changes: 14 additions & 55 deletions src/inotify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ use std::{
io,
os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd},
path::Path,
sync::{atomic::AtomicBool, Arc},
sync::Arc,
};

use inotify_sys as ffi;
use libc::{fcntl, F_GETFL, F_SETFL, O_NONBLOCK};

use crate::events::Events;
use crate::fd_guard::FdGuard;
use crate::util::read_into_buffer;
use crate::watches::{WatchDescriptor, WatchMask, Watches};

Expand All @@ -28,7 +27,7 @@ use crate::stream::EventStream;
/// [top-level documentation]: crate
#[derive(Debug)]
pub struct Inotify {
fd: Arc<FdGuard>,
fd: Arc<OwnedFd>,
}

impl Inotify {
Expand Down Expand Up @@ -85,12 +84,10 @@ impl Inotify {
return Err(io::Error::last_os_error());
}

Ok(Inotify {
fd: Arc::new(FdGuard {
fd,
close_on_drop: AtomicBool::new(true),
}),
})
// SAFETY: The resource pointed to by fd is open and suitable for assuming ownership.
let fd = unsafe { OwnedFd::from_raw_fd(fd) };

Ok(Inotify { fd: Arc::new(fd) })
}

/// Gets an interface that allows adding and removing watches.
Expand Down Expand Up @@ -124,21 +121,21 @@ impl Inotify {
/// the documentation of [`Inotify::read_events`] for more information.
pub fn read_events_blocking<'a>(&mut self, buffer: &'a mut [u8]) -> io::Result<Events<'a>> {
unsafe {
let res = fcntl(**self.fd, F_GETFL);
let res = fcntl(self.fd.as_raw_fd(), F_GETFL);
if res == -1 {
return Err(io::Error::last_os_error());
}
if fcntl(**self.fd, F_SETFL, res & !O_NONBLOCK) == -1 {
if fcntl(self.fd.as_raw_fd(), F_SETFL, res & !O_NONBLOCK) == -1 {
return Err(io::Error::last_os_error());
}
};
let result = self.read_events(buffer);
unsafe {
let res = fcntl(**self.fd, F_GETFL);
let res = fcntl(self.fd.as_raw_fd(), F_GETFL);
if res == -1 {
return Err(io::Error::last_os_error());
}
if fcntl(**self.fd, F_SETFL, res | O_NONBLOCK) == -1 {
if fcntl(self.fd.as_raw_fd(), F_SETFL, res | O_NONBLOCK) == -1 {
return Err(io::Error::last_os_error());
}
};
Expand Down Expand Up @@ -198,7 +195,7 @@ impl Inotify {
/// [`ErrorKind::UnexpectedEof`]: std::io::ErrorKind::UnexpectedEof
/// [`ErrorKind::InvalidInput`]: std::io::ErrorKind::InvalidInput
pub fn read_events<'a>(&mut self, buffer: &'a mut [u8]) -> io::Result<Events<'a>> {
let num_bytes = read_into_buffer(**self.fd, buffer);
let num_bytes = read_into_buffer(self.fd.as_raw_fd(), buffer);

let num_bytes = match num_bytes {
0 => {
Expand Down Expand Up @@ -269,46 +266,9 @@ impl Inotify {
/// initialized in `Inotify::init`. This is intended to be used to transform an
/// `EventStream` back into an `Inotify`. Do not attempt to clone `Inotify` with this.
#[cfg(feature = "stream")]
pub(crate) fn from_file_descriptor(fd: Arc<FdGuard>) -> Self {
pub(crate) fn from_file_descriptor(fd: Arc<OwnedFd>) -> Self {
Inotify { fd }
}

/// Closes the inotify instance
///
/// Closes the file descriptor referring to the inotify instance. The user
/// usually doesn't have to call this function, as the underlying inotify
/// instance is closed automatically, when [`Inotify`] is dropped.
///
/// # Errors
///
/// Directly returns the error from the call to [`close`], without adding any
/// error conditions of its own.
///
/// # Examples
///
/// ```
/// use inotify::Inotify;
///
/// let mut inotify = Inotify::init()
/// .expect("Failed to initialize an inotify instance");
///
/// inotify.close()
/// .expect("Failed to close inotify instance");
/// ```
///
/// [`close`]: libc::close
pub fn close(self) -> io::Result<()> {
// `self` will be dropped when this method returns. If this is the only
// owner of `fd`, the `Arc` will also be dropped. The `Drop`
// implementation for `FdGuard` will attempt to close the file descriptor
// again, unless this flag here is cleared.
self.fd.should_not_close();

match unsafe { ffi::close(**self.fd) } {
0 => Ok(()),
_ => Err(io::Error::last_os_error()),
}
}
}

impl AsRawFd for Inotify {
Expand All @@ -321,16 +281,15 @@ impl AsRawFd for Inotify {
impl FromRawFd for Inotify {
unsafe fn from_raw_fd(fd: RawFd) -> Self {
Inotify {
fd: Arc::new(FdGuard::from_raw_fd(fd)),
fd: Arc::new(OwnedFd::from_raw_fd(fd)),
}
}
}

impl IntoRawFd for Inotify {
#[inline]
fn into_raw_fd(self) -> RawFd {
self.fd.should_not_close();
self.fd.fd
self.fd.as_raw_fd()
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
//! ```
//! When you want to read events asynchronously, you need to convert it to [`EventStream`].
//! The transform function is [`Inotify::into_event_stream`]
//! ```
//! ```skip
//! let mut stream = inotify.into_event_stream(&mut buffer)?;
//! // Read events from async stream
//! while let Some(event_or_error) = stream.next().await {
Expand Down Expand Up @@ -83,7 +83,6 @@
extern crate bitflags;

mod events;
mod fd_guard;
mod inotify;
mod util;
mod watches;
Expand Down
9 changes: 4 additions & 5 deletions src/stream.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{
io,
os::unix::io::AsRawFd,
os::fd::{AsRawFd as _, OwnedFd},
pin::Pin,
sync::Arc,
task::{Context, Poll},
Expand All @@ -10,7 +10,6 @@ use futures_core::{ready, Stream};
use tokio::io::unix::AsyncFd;

use crate::events::{Event, EventOwned};
use crate::fd_guard::FdGuard;
use crate::util::read_into_buffer;
use crate::watches::Watches;
use crate::Inotify;
Expand All @@ -20,7 +19,7 @@ use crate::Inotify;
/// Allows for streaming events returned by [`Inotify::into_event_stream`].
#[derive(Debug)]
pub struct EventStream<T> {
fd: AsyncFd<Arc<FdGuard>>,
fd: AsyncFd<Arc<OwnedFd>>,
buffer: T,
buffer_pos: usize,
unused_bytes: usize,
Expand All @@ -31,7 +30,7 @@ where
T: AsMut<[u8]> + AsRef<[u8]>,
{
/// Returns a new `EventStream` associated with the default reactor.
pub(crate) fn new(fd: Arc<FdGuard>, buffer: T) -> io::Result<Self> {
pub(crate) fn new(fd: Arc<OwnedFd>, buffer: T) -> io::Result<Self> {
Ok(EventStream {
fd: AsyncFd::new(fd)?,
buffer,
Expand Down Expand Up @@ -90,7 +89,7 @@ where
}

fn read(
fd: &AsyncFd<Arc<FdGuard>>,
fd: &AsyncFd<Arc<OwnedFd>>,
buffer: &mut [u8],
cx: &mut Context,
) -> Poll<io::Result<usize>> {
Expand Down
Loading

0 comments on commit ad90da8

Please sign in to comment.