Use io-lifetimes types in API, instead of RawFd

Operating on `RawFd`s is unsafe, so this should be changed. Though some
uses will be cleaner once `nix` is updated for IO safety (which seems
to be ongoing, but complicated).

The `Rc<RefCell<IoDispatcher>>` in `Async` is somewhat awkward here. For
now `Dispatcher` still contains a `RawFd`. The only alternative I can
think of with how the API works is to use `Rc<F>` and have `into_inner`
call `Rc::try_unwrap(self.fd).unwrap()`.

Author: ids1024

Cargo.toml

@@ -18,6 +18,7 @@ members = [ "doc" ]
 codecov = { repository = "Smithay/calloop" }
 
 [dependencies]
+io-lifetimes = "1.0.3"
 log = "0.4"
 nix = { version = "0.24", default-features = false, features = ["event", "fs", "signal", "socket", "time"] }
 futures-util = { version = "0.3.5", optional = true, default-features = false, features = ["std"]}

src/io.rs

@@ -7,11 +7,12 @@
 //! [`LoopHandle::adapt_io`]: crate::LoopHandle#method.adapt_io
 
 use std::cell::RefCell;
-use std::os::unix::io::{AsRawFd, RawFd};
+use std::os::unix::io::{AsRawFd, BorrowedFd, RawFd};
 use std::pin::Pin;
 use std::rc::Rc;
 use std::task::{Context, Poll as TaskPoll, Waker};
 
+use io_lifetimes::AsFd;
 use nix::fcntl::{fcntl, FcntlArg, OFlag};
 
 #[cfg(feature = "futures-io")]
@@ -31,30 +32,30 @@ use crate::{
 /// `AsyncWrite` if the underlying type implements `Read` and/or `Write`.
 ///
 /// Note that this adapter and the futures procuded from it and *not* threadsafe.
-pub struct Async<'l, F: AsRawFd> {
+pub struct Async<'l, F: AsFd> {
     fd: Option<F>,
     dispatcher: Rc<RefCell<IoDispatcher>>,
     inner: Rc<dyn IoLoopInner + 'l>,
     old_flags: OFlag,
 }
 
-impl<'l, F: AsRawFd + std::fmt::Debug> std::fmt::Debug for Async<'l, F> {
+impl<'l, F: AsFd + std::fmt::Debug> std::fmt::Debug for Async<'l, F> {
     #[cfg_attr(coverage, no_coverage)]
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("Async").field("fd", &self.fd).finish()
     }
 }
 
-impl<'l, F: AsRawFd> Async<'l, F> {
+impl<'l, F: AsFd> Async<'l, F> {
     pub(crate) fn new<Data>(inner: Rc<LoopInner<'l, Data>>, fd: F) -> crate::Result<Async<'l, F>> {
-        let rawfd = fd.as_raw_fd();
+        let rawfd = fd.as_fd().as_raw_fd();
         // set non-blocking
         let old_flags = fcntl(rawfd, FcntlArg::F_GETFL)?;
         let old_flags = unsafe { OFlag::from_bits_unchecked(old_flags) };
         fcntl(rawfd, FcntlArg::F_SETFL(old_flags | OFlag::O_NONBLOCK))?;
         // register in the loop
         let dispatcher = Rc::new(RefCell::new(IoDispatcher {
-            fd: rawfd,
+            fd: fd.as_fd().as_raw_fd(),
             token: None,
             waker: None,
             is_registered: false,
@@ -114,11 +115,11 @@ impl<'l, F: AsRawFd> Async<'l, F> {
 
 /// A future that resolves once the associated object becomes ready for reading
 #[derive(Debug)]
-pub struct Readable<'s, 'l, F: AsRawFd> {
+pub struct Readable<'s, 'l, F: AsFd> {
     io: &'s mut Async<'l, F>,
 }
 
-impl<'s, 'l, F: AsRawFd> std::future::Future for Readable<'s, 'l, F> {
+impl<'s, 'l, F: AsFd> std::future::Future for Readable<'s, 'l, F> {
     type Output = ();
     fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> TaskPoll<()> {
         let io = &mut self.as_mut().io;
@@ -134,11 +135,11 @@ impl<'s, 'l, F: AsRawFd> std::future::Future for Readable<'s, 'l, F> {
 
 /// A future that resolves once the associated object becomes ready for writing
 #[derive(Debug)]
-pub struct Writable<'s, 'l, F: AsRawFd> {
+pub struct Writable<'s, 'l, F: AsFd> {
     io: &'s mut Async<'l, F>,
 }
 
-impl<'s, 'l, F: AsRawFd> std::future::Future for Writable<'s, 'l, F> {
+impl<'s, 'l, F: AsFd> std::future::Future for Writable<'s, 'l, F> {
     type Output = ();
     fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> TaskPoll<()> {
         let io = &mut self.as_mut().io;
@@ -152,7 +153,7 @@ impl<'s, 'l, F: AsRawFd> std::future::Future for Writable<'s, 'l, F> {
     }
 }
 
-impl<'l, F: AsRawFd> Drop for Async<'l, F> {
+impl<'l, F: AsFd> Drop for Async<'l, F> {
     fn drop(&mut self) {
         self.inner.kill(&self.dispatcher);
         // restore flags
@@ -163,7 +164,7 @@ impl<'l, F: AsRawFd> Drop for Async<'l, F> {
     }
 }
 
-impl<'l, F: AsRawFd> Unpin for Async<'l, F> {}
+impl<'l, F: AsFd> Unpin for Async<'l, F> {}
 
 trait IoLoopInner {
     fn register(&self, dispatcher: &RefCell<IoDispatcher>) -> crate::Result<()>;
@@ -175,7 +176,7 @@ impl<'l, Data> IoLoopInner for LoopInner<'l, Data> {
     fn register(&self, dispatcher: &RefCell<IoDispatcher>) -> crate::Result<()> {
         let disp = dispatcher.borrow();
         self.poll.borrow_mut().register(
-            disp.fd,
+            unsafe { BorrowedFd::borrow_raw(disp.fd) },
             Interest::EMPTY,
             Mode::OneShot,
             disp.token.expect("No token for IO dispatcher"),
@@ -185,7 +186,7 @@ impl<'l, Data> IoLoopInner for LoopInner<'l, Data> {
     fn reregister(&self, dispatcher: &RefCell<IoDispatcher>) -> crate::Result<()> {
         let disp = dispatcher.borrow();
         self.poll.borrow_mut().reregister(
-            disp.fd,
+            unsafe { BorrowedFd::borrow_raw(disp.fd) },
             disp.interest,
             Mode::OneShot,
             disp.token.expect("No token for IO dispatcher"),
@@ -207,7 +208,7 @@ impl<'l, Data> IoLoopInner for LoopInner<'l, Data> {
 }
 
 struct IoDispatcher {
-    fd: RawFd,
+    fd: RawFd, // FIXME: `BorrowedFd`? How to statically verify it doesn't outlive file?
     token: Option<Token>,
     waker: Option<Waker>,
     is_registered: bool,
@@ -249,7 +250,7 @@ impl<Data> EventDispatcher<Data> for RefCell<IoDispatcher> {
     fn unregister(&self, poll: &mut Poll) -> crate::Result<bool> {
         let disp = self.borrow();
         if disp.is_registered {
-            poll.unregister(disp.fd)?;
+            poll.unregister(unsafe { BorrowedFd::borrow_raw(disp.fd) })?;
         }
         Ok(true)
     }
@@ -268,7 +269,7 @@ impl<Data> EventDispatcher<Data> for RefCell<IoDispatcher> {
 
 #[cfg(feature = "futures-io")]
 #[cfg_attr(docsrs, doc(cfg(feature = "futures-io")))]
-impl<'l, F: AsRawFd + std::io::Read> AsyncRead for Async<'l, F> {
+impl<'l, F: AsFd + std::io::Read> AsyncRead for Async<'l, F> {
     fn poll_read(
         mut self: Pin<&mut Self>,
         cx: &mut Context<'_>,
@@ -298,7 +299,7 @@ impl<'l, F: AsRawFd + std::io::Read> AsyncRead for Async<'l, F> {
 
 #[cfg(feature = "futures-io")]
 #[cfg_attr(docsrs, doc(cfg(feature = "futures-io")))]
-impl<'l, F: AsRawFd + std::io::Write> AsyncWrite for Async<'l, F> {
+impl<'l, F: AsFd + std::io::Write> AsyncWrite for Async<'l, F> {
     fn poll_write(
         mut self: Pin<&mut Self>,
         cx: &mut Context<'_>,

src/loop_logic.rs

@@ -1,12 +1,12 @@
 use std::cell::{Cell, RefCell};
 use std::fmt::Debug;
 use std::io;
-use std::os::unix::io::AsRawFd;
 use std::rc::Rc;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 
+use io_lifetimes::AsFd;
 use slotmap::SlotMap;
 
 use crate::sources::{Dispatcher, EventSource, Idle, IdleDispatcher};
@@ -194,7 +194,7 @@ impl<'l, Data> LoopHandle<'l, Data> {
     ///
     /// The produced futures can be polled in any executor, and notably the one provided by
     /// calloop.
-    pub fn adapt_io<F: AsRawFd>(&self, fd: F) -> crate::Result<crate::io::Async<'l, F>> {
+    pub fn adapt_io<F: AsFd>(&self, fd: F) -> crate::Result<crate::io::Async<'l, F>> {
         crate::io::Async::new(self.inner.clone(), fd)
     }
 }
@@ -578,9 +578,12 @@ mod tests {
 
     #[test]
     fn insert_bad_source() {
+        use std::os::unix::io::FromRawFd;
+
         let event_loop = EventLoop::<()>::try_new().unwrap();
+        let fd = unsafe { io_lifetimes::OwnedFd::from_raw_fd(420) };
         let ret = event_loop.handle().insert_source(
-            crate::sources::generic::Generic::new(420, Interest::READ, Mode::Level),
+            crate::sources::generic::Generic::new(fd, Interest::READ, Mode::Level),
             |_, _, _| Ok(PostAction::Continue),
         );
         assert!(ret.is_err());
@@ -589,9 +592,11 @@ mod tests {
     #[test]
     fn insert_source_no_interest() {
         use nix::unistd::{close, pipe};
+        use std::os::unix::io::FromRawFd;
 
         // Create a pipe to get an arbitrary fd.
         let (read, write) = pipe().unwrap();
+        let read = unsafe { io_lifetimes::OwnedFd::from_raw_fd(read) };
         // We don't need the write end.
         close(write).unwrap();
 
@@ -605,7 +610,6 @@ mod tests {
         if let Ok(token) = ret {
             // Unwrap the dispatcher+source and close the read end.
             handle.remove(token);
-            close(dispatcher.into_source_inner().unwrap()).unwrap();
         } else {
             // Fail the test.
             panic!();
@@ -753,6 +757,7 @@ mod tests {
     fn change_interests() {
         use nix::sys::socket::{recv, socketpair, AddressFamily, MsgFlags, SockFlag, SockType};
         use nix::unistd::write;
+        use std::os::unix::io::{AsRawFd, FromRawFd};
         let mut event_loop = EventLoop::<bool>::try_new().unwrap();
 
         let (sock1, sock2) = socketpair(
@@ -762,14 +767,15 @@ mod tests {
             SockFlag::empty(), // recv with DONTWAIT will suffice for platforms without SockFlag::SOCK_NONBLOCKING such as macOS
         )
         .unwrap();
+        let sock1 = unsafe { io_lifetimes::OwnedFd::from_raw_fd(sock1) };
 
         let source = Generic::new(sock1, Interest::READ, Mode::Level);
-        let dispatcher = Dispatcher::new(source, |_, &mut fd, dispatched| {
+        let dispatcher = Dispatcher::new(source, |_, fd, dispatched| {
             *dispatched = true;
             // read all contents available to drain the socket
             let mut buf = [0u8; 32];
             loop {
-                match recv(fd, &mut buf, MsgFlags::MSG_DONTWAIT) {
+                match recv(fd.as_raw_fd(), &mut buf, MsgFlags::MSG_DONTWAIT) {
                     Ok(0) => break, // closed pipe, we are now inert
                     Ok(_) => {}
                     Err(e) => {

src/sources/generic.rs

@@ -15,7 +15,7 @@
 //! # let mut event_loop = calloop::EventLoop::<()>::try_new()
 //! #                .expect("Failed to initialize the event loop!");
 //! # let handle = event_loop.handle();
-//! # let io_object = 0;
+//! # let io_object = std::io::stdin();
 //! handle.insert_source(
 //!     // wrap your IO object in a Generic, here we register for read readiness
 //!     // in level-triggering mode
@@ -36,17 +36,15 @@
 //! It can also help you implementing your own event sources: just have
 //! these `Generic<_>` as fields of your event source, and delegate the
 //! [`EventSource`](crate::EventSource) implementation to them.
-//!
-//! If you need to directly work with a [`RawFd`](std::os::unix::io::RawFd), rather than an
-//! FD-backed object, see [`Generic::from_fd`](Generic#method.from_fd).
 
-use std::{marker::PhantomData, os::unix::io::AsRawFd};
+use io_lifetimes::AsFd;
+use std::marker::PhantomData;
 
 use crate::{EventSource, Interest, Mode, Poll, PostAction, Readiness, Token, TokenFactory};
 
 /// A generic event source wrapping a FD-backed type
 #[derive(Debug)]
-pub struct Generic<F: AsRawFd, E = std::io::Error> {
+pub struct Generic<F: AsFd, E = std::io::Error> {
     /// The wrapped FD-backed type
     pub file: F,
     /// The programmed interest
@@ -62,7 +60,7 @@ pub struct Generic<F: AsRawFd, E = std::io::Error> {
     _error_type: PhantomData<E>,
 }
 
-impl<F: AsRawFd> Generic<F, std::io::Error> {
+impl<F: AsFd> Generic<F, std::io::Error> {
     /// Wrap a FD-backed type into a `Generic` event source that uses
     /// [`std::io::Error`] as its error type.
     pub fn new(file: F, interest: Interest, mode: Mode) -> Generic<F, std::io::Error> {
@@ -87,7 +85,7 @@ impl<F: AsRawFd> Generic<F, std::io::Error> {
     }
 }
 
-impl<F: AsRawFd, E> Generic<F, E> {
+impl<F: AsFd, E> Generic<F, E> {
     /// Unwrap the `Generic` source to retrieve the underlying type
     pub fn unwrap(self) -> F {
         self.file
@@ -96,7 +94,7 @@ impl<F: AsRawFd, E> Generic<F, E> {
 
 impl<F, E> EventSource for Generic<F, E>
 where
-    F: AsRawFd,
+    F: AsFd,
     E: Into<Box<dyn std::error::Error + Send + Sync>>,
 {
     type Event = Readiness;
@@ -124,7 +122,7 @@ where
     fn register(&mut self, poll: &mut Poll, token_factory: &mut TokenFactory) -> crate::Result<()> {
         let token = token_factory.token();
 
-        poll.register(self.file.as_raw_fd(), self.interest, self.mode, token)?;
+        poll.register(self.file.as_fd(), self.interest, self.mode, token)?;
 
         self.token = Some(token);
         Ok(())
@@ -137,14 +135,14 @@ where
     ) -> crate::Result<()> {
         let token = token_factory.token();
 
-        poll.reregister(self.file.as_raw_fd(), self.interest, self.mode, token)?;
+        poll.reregister(self.file.as_fd(), self.interest, self.mode, token)?;
 
         self.token = Some(token);
         Ok(())
     }
 
     fn unregister(&mut self, poll: &mut Poll) -> crate::Result<()> {
-        poll.unregister(self.file.as_raw_fd())?;
+        poll.unregister(self.file.as_fd())?;
         self.token = None;
         Ok(())
     }

src/sources/ping.rs

@@ -9,9 +9,6 @@
 //! block to construct event sources whose source of event is not file descriptor, but rather an
 //! userspace source (like an other thread).
 
-use nix::unistd::close;
-use std::os::unix::io::RawFd;
-
 // The ping source has platform-dependent implementations provided by modules
 // under this one. These modules should expose:
 // - a make_ping() function
@@ -58,17 +55,6 @@ pub type PingSource = platform::PingSource;
 #[error(transparent)]
 pub struct PingError(Box<dyn std::error::Error + Sync + Send>);
 
-#[derive(Debug)]
-struct CloseOnDrop(RawFd);
-
-impl Drop for CloseOnDrop {
-    fn drop(&mut self) {
-        if let Err(e) = close(self.0) {
-            log::warn!("[calloop] Failed to close ping fd: {:?}", e);
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use crate::transient::TransientSource;