Replace FdGuard with OwnedFd

src/events.rs

@@ -1,13 +1,11 @@
 use std::{
     ffi::{OsStr, OsString},
     mem,
-    os::unix::ffi::OsStrExt,
-    sync::Weak,
+    os::{fd::BorrowedFd, unix::ffi::OsStrExt},
 };
 
 use inotify_sys as ffi;
 
-use crate::fd_guard::FdGuard;
 use crate::watches::WatchDescriptor;
 
 /// Iterator over inotify events
@@ -18,15 +16,15 @@ use crate::watches::WatchDescriptor;
 /// [`Inotify::read_events_blocking`]: crate::Inotify::read_events_blocking
 /// [`Inotify::read_events`]: crate::Inotify::read_events
 #[derive(Debug)]
-pub struct Events<'a> {
-    fd: Weak<FdGuard>,
+pub struct Events<'a, 'i> {
+    fd: BorrowedFd<'i>,
     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 {
+impl<'a, 'i> Events<'a, 'i> {
+    pub(crate) fn new(fd: BorrowedFd<'i>, buffer: &'a [u8], num_bytes: usize) -> Self {
         Events {
             fd,
             buffer,
@@ -36,8 +34,8 @@ impl<'a> Events<'a> {
     }
 }
 
-impl<'a> Iterator for Events<'a> {
-    type Item = Event<&'a OsStr>;
+impl<'a, 'i> Iterator for Events<'a, 'i> {
+    type Item = Event<'i, &'a OsStr>;
 
     fn next(&mut self) -> Option<Self::Item> {
         if self.pos < self.num_bytes {
@@ -62,7 +60,7 @@ impl<'a> Iterator for Events<'a> {
 /// [`Inotify::read_events_blocking`]: crate::Inotify::read_events_blocking
 /// [`Inotify::read_events`]: crate::Inotify::read_events
 #[derive(Clone, Debug)]
-pub struct Event<S> {
+pub struct Event<'i, S> {
     /// Identifies the watch this event originates from
     ///
     /// This [`WatchDescriptor`] is equal to the one that [`Watches::add`]
@@ -73,7 +71,7 @@ pub struct Event<S> {
     ///
     /// [`Watches::add`]: crate::Watches::add
     /// [`Watches::remove`]: crate::Watches::remove
-    pub wd: WatchDescriptor,
+    pub wd: WatchDescriptor<'i>,
 
     /// Indicates what kind of event this is
     pub mask: EventMask,
@@ -96,8 +94,8 @@ pub struct Event<S> {
     pub name: Option<S>,
 }
 
-impl<'a> Event<&'a OsStr> {
-    fn new(fd: Weak<FdGuard>, event: &ffi::inotify_event, name: &'a OsStr) -> Self {
+impl<'a, 'i> Event<'i, &'a OsStr> {
+    fn new(fd: BorrowedFd<'i>, 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.");
 
@@ -123,7 +121,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: BorrowedFd<'i>, 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
@@ -177,7 +175,7 @@ impl<'a> Event<&'a OsStr> {
 
     /// Returns an owned copy of the event.
     #[must_use = "cloning is often expensive and is not expected to have side effects"]
-    pub fn to_owned(&self) -> EventOwned {
+    pub fn to_owned(&self) -> Event<'i, OsString> {
         Event {
             wd: self.wd.clone(),
             mask: self.mask,
@@ -188,7 +186,7 @@ impl<'a> Event<&'a OsStr> {
 }
 
 /// An owned version of `Event`
-pub type EventOwned = Event<OsString>;
+pub type EventOwned<'i> = Event<'i, OsString>;
 
 bitflags! {
     /// Indicates the type of an event
@@ -340,7 +338,7 @@ impl EventMask {
 
 #[cfg(test)]
 mod tests {
-    use std::{io::prelude::*, mem, slice, sync};
+    use std::{fs::File, io::prelude::*, mem, os::fd::AsFd as _, slice};
 
     use inotify_sys as ffi;
 
@@ -367,9 +365,11 @@ mod tests {
         // After that event, simulate an event that starts with a non-zero byte.
         buffer[mem::size_of_val(event)] = 1;
 
+        let file = File::open("/dev/null").unwrap();
+        let fd = file.as_fd();
         // Now create the event and verify that the name is actually `None`, as
         // dictated by the value `len` above.
-        let (_, event) = Event::from_buffer(sync::Weak::new(), &buffer);
+        let (_, event) = Event::from_buffer(fd, &buffer);
         assert_eq!(event.name, None);
     }
 }

src/inotify.rs

@@ -2,14 +2,12 @@ use std::{
     io,
     os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd},
     path::Path,
-    sync::{atomic::AtomicBool, 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};
 
@@ -28,7 +26,7 @@ use crate::stream::EventStream;
 /// [top-level documentation]: crate
 #[derive(Debug)]
 pub struct Inotify {
-    fd: Arc<FdGuard>,
+    fd: OwnedFd,
 }
 
 impl Inotify {
@@ -85,18 +83,16 @@ 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 })
     }
 
     /// Gets an interface that allows adding and removing watches.
     /// See [`Watches::add`] and [`Watches::remove`].
     pub fn watches(&self) -> Watches {
-        Watches::new(self.fd.clone())
+        Watches::new(self.fd.as_fd())
     }
 
     /// Deprecated: use `Inotify.watches().add()` instead
@@ -122,23 +118,27 @@ impl Inotify {
     /// This method calls [`Inotify::read_events`] internally and behaves
     /// essentially the same, apart from the blocking behavior. Please refer to
     /// 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>> {
+    pub fn read_events_blocking<'a, 'i>(
+        &'i mut self,
+        buffer: &'a mut [u8],
+    ) -> io::Result<Events<'a, 'i>> {
         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 raw_fd = self.fd.as_raw_fd();
         let result = self.read_events(buffer);
         unsafe {
-            let res = fcntl(**self.fd, F_GETFL);
+            let res = fcntl(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(raw_fd, F_SETFL, res | O_NONBLOCK) == -1 {
                 return Err(io::Error::last_os_error());
             }
         };
@@ -197,8 +197,8 @@ impl Inotify {
     /// [`read`]: libc::read
     /// [`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);
+    pub fn read_events<'a, 'i>(&'i mut self, buffer: &'a mut [u8]) -> io::Result<Events<'a, 'i>> {
+        let num_bytes = read_into_buffer(self.fd.as_raw_fd(), buffer);
 
         let num_bytes = match num_bytes {
             0 => {
@@ -236,19 +236,7 @@ impl Inotify {
             }
         };
 
-        Ok(Events::new(Arc::downgrade(&self.fd), buffer, num_bytes))
-    }
-
-    /// Deprecated: use `into_event_stream()` instead, which enforces a single `Stream` and predictable reads.
-    /// Using this method to create multiple `EventStream` instances from one `Inotify` is unsupported,
-    /// as they will contend over one event source and each produce unpredictable stream contents.
-    #[deprecated = "use `into_event_stream()` instead, which enforces a single Stream and predictable reads"]
-    #[cfg(feature = "stream")]
-    pub fn event_stream<T>(&mut self, buffer: T) -> io::Result<EventStream<T>>
-    where
-        T: AsMut<[u8]> + AsRef<[u8]>,
-    {
-        EventStream::new(self.fd.clone(), buffer)
+        Ok(Events::new(self.fd.as_fd(), buffer, num_bytes))
     }
 
     /// Create a stream which collects events. Consumes the `Inotify` instance.
@@ -269,46 +257,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: 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 {
@@ -321,16 +272,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: 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.into_raw_fd()
     }
 }
 

src/lib.rs

@@ -83,7 +83,6 @@
 extern crate bitflags;
 
 mod events;
-mod fd_guard;
 mod inotify;
 mod util;
 mod watches;