Replace FdGuard with OwnedFd

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.

Events and Watches now have a lifetime referencing the original file
descriptor, since they have a BorrowedFd referencing it.

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;