diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs index 34d567c21..dc53b4341 100644 --- a/src/sys/unix/mod.rs +++ b/src/sys/unix/mod.rs @@ -4,6 +4,7 @@ #[allow(unused_macros)] macro_rules! syscall { ($fn: ident ( $($arg: expr),* $(,)* ) ) => {{ + #[allow(unused_unsafe)] let res = unsafe { libc::$fn($($arg, )*) }; if res < 0 { Err(std::io::Error::last_os_error()) diff --git a/src/sys/unix/selector/epoll.rs b/src/sys/unix/selector/epoll.rs index d4709d314..de6cff381 100644 --- a/src/sys/unix/selector/epoll.rs +++ b/src/sys/unix/selector/epoll.rs @@ -1,4 +1,4 @@ -use std::os::fd::{AsRawFd, RawFd}; +use std::os::fd::{AsRawFd, FromRawFd, OwnedFd, RawFd}; #[cfg(debug_assertions)] use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Duration; @@ -16,20 +16,22 @@ static NEXT_ID: AtomicUsize = AtomicUsize::new(1); pub struct Selector { #[cfg(debug_assertions)] id: usize, - ep: RawFd, + ep: OwnedFd, } impl Selector { pub fn new() -> io::Result { + // SAFETY: `epoll_create1(2)` ensures the fd is valid. + let ep = unsafe { OwnedFd::from_raw_fd(syscall!(epoll_create1(libc::EPOLL_CLOEXEC))?) }; Ok(Selector { #[cfg(debug_assertions)] id: NEXT_ID.fetch_add(1, Ordering::Relaxed), - ep: syscall!(epoll_create1(libc::EPOLL_CLOEXEC))?, + ep, }) } pub fn try_clone(&self) -> io::Result { - syscall!(fcntl(self.ep, libc::F_DUPFD_CLOEXEC, super::LOWEST_FD)).map(|ep| Selector { + self.ep.try_clone().map(|ep| Selector { // It's the same selector, so we use the same id. #[cfg(debug_assertions)] id: self.id, @@ -52,7 +54,7 @@ impl Selector { events.clear(); syscall!(epoll_wait( - self.ep, + self.ep.as_raw_fd(), events.as_mut_ptr(), events.capacity() as i32, timeout, @@ -72,7 +74,8 @@ impl Selector { _pad: 0, }; - syscall!(epoll_ctl(self.ep, libc::EPOLL_CTL_ADD, fd, &mut event)).map(|_| ()) + let ep = self.ep.as_raw_fd(); + syscall!(epoll_ctl(ep, libc::EPOLL_CTL_ADD, fd, &mut event)).map(|_| ()) } pub fn reregister(&self, fd: RawFd, token: Token, interests: Interest) -> io::Result<()> { @@ -83,11 +86,13 @@ impl Selector { _pad: 0, }; - syscall!(epoll_ctl(self.ep, libc::EPOLL_CTL_MOD, fd, &mut event)).map(|_| ()) + let ep = self.ep.as_raw_fd(); + syscall!(epoll_ctl(ep, libc::EPOLL_CTL_MOD, fd, &mut event)).map(|_| ()) } pub fn deregister(&self, fd: RawFd) -> io::Result<()> { - syscall!(epoll_ctl(self.ep, libc::EPOLL_CTL_DEL, fd, ptr::null_mut())).map(|_| ()) + let ep = self.ep.as_raw_fd(); + syscall!(epoll_ctl(ep, libc::EPOLL_CTL_DEL, fd, ptr::null_mut())).map(|_| ()) } } @@ -102,15 +107,7 @@ cfg_io_source! { impl AsRawFd for Selector { fn as_raw_fd(&self) -> RawFd { - self.ep - } -} - -impl Drop for Selector { - fn drop(&mut self) { - if let Err(err) = syscall!(close(self.ep)) { - error!("error closing epoll: {}", err); - } + self.ep.as_raw_fd() } } diff --git a/src/sys/unix/selector/kqueue.rs b/src/sys/unix/selector/kqueue.rs index 112651772..1417f92a6 100644 --- a/src/sys/unix/selector/kqueue.rs +++ b/src/sys/unix/selector/kqueue.rs @@ -1,7 +1,7 @@ use crate::{Interest, Token}; use std::mem::{self, MaybeUninit}; use std::ops::{Deref, DerefMut}; -use std::os::fd::{AsRawFd, RawFd}; +use std::os::fd::{AsRawFd, FromRawFd, OwnedFd, RawFd}; #[cfg(debug_assertions)] use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Duration; @@ -65,24 +65,23 @@ macro_rules! kevent { pub struct Selector { #[cfg(debug_assertions)] id: usize, - kq: RawFd, + kq: OwnedFd, } impl Selector { pub fn new() -> io::Result { - let kq = syscall!(kqueue())?; - let selector = Selector { + // SAFETY: `kqueue(2)` ensures the fd is valid. + let kq = unsafe { OwnedFd::from_raw_fd(syscall!(kqueue())?) }; + syscall!(fcntl(kq.as_raw_fd(), libc::F_SETFD, libc::FD_CLOEXEC))?; + Ok(Selector { #[cfg(debug_assertions)] id: NEXT_ID.fetch_add(1, Ordering::Relaxed), kq, - }; - - syscall!(fcntl(kq, libc::F_SETFD, libc::FD_CLOEXEC))?; - Ok(selector) + }) } pub fn try_clone(&self) -> io::Result { - syscall!(fcntl(self.kq, libc::F_DUPFD_CLOEXEC, super::LOWEST_FD)).map(|kq| Selector { + self.kq.try_clone().map(|kq| Selector { // It's the same selector, so we use the same id. #[cfg(debug_assertions)] id: self.id, @@ -106,7 +105,7 @@ impl Selector { events.clear(); syscall!(kevent( - self.kq, + self.kq.as_raw_fd(), ptr::null(), 0, events.as_mut_ptr(), @@ -156,7 +155,7 @@ impl Selector { // the array. slice::from_raw_parts_mut(changes[0].as_mut_ptr(), n_changes) }; - kevent_register(self.kq, changes, &[libc::EPIPE as i64]) + kevent_register(self.kq.as_raw_fd(), changes, &[libc::EPIPE as i64]) } pub fn reregister(&self, fd: RawFd, token: Token, interests: Interest) -> io::Result<()> { @@ -186,7 +185,7 @@ impl Selector { // // For the explanation of ignoring `EPIPE` see `register`. kevent_register( - self.kq, + self.kq.as_raw_fd(), &mut changes, &[libc::ENOENT as i64, libc::EPIPE as i64], ) @@ -204,7 +203,7 @@ impl Selector { // the ENOENT error when it comes up. The ENOENT error informs us that // the filter wasn't there in first place, but we don't really care // about that since our goal is to remove it. - kevent_register(self.kq, &mut changes, &[libc::ENOENT as i64]) + kevent_register(self.kq.as_raw_fd(), &mut changes, &[libc::ENOENT as i64]) } // Used by `Waker`. @@ -224,7 +223,8 @@ impl Selector { token.0 ); - syscall!(kevent(self.kq, &kevent, 1, &mut kevent, 1, ptr::null())).and_then(|_| { + let kq = self.kq.as_raw_fd(); + syscall!(kevent(kq, &kevent, 1, &mut kevent, 1, ptr::null())).and_then(|_| { if (kevent.flags & libc::EV_ERROR) != 0 && kevent.data != 0 { Err(io::Error::from_raw_os_error(kevent.data as i32)) } else { @@ -250,7 +250,8 @@ impl Selector { ); kevent.fflags = libc::NOTE_TRIGGER; - syscall!(kevent(self.kq, &kevent, 1, &mut kevent, 1, ptr::null())).and_then(|_| { + let kq = self.kq.as_raw_fd(); + syscall!(kevent(kq, &kevent, 1, &mut kevent, 1, ptr::null())).and_then(|_| { if (kevent.flags & libc::EV_ERROR) != 0 && kevent.data != 0 { Err(io::Error::from_raw_os_error(kevent.data as i32)) } else { @@ -314,15 +315,7 @@ cfg_io_source! { impl AsRawFd for Selector { fn as_raw_fd(&self) -> RawFd { - self.kq - } -} - -impl Drop for Selector { - fn drop(&mut self) { - if let Err(err) = syscall!(close(self.kq)) { - error!("error closing kqueue: {}", err); - } + self.kq.as_raw_fd() } } diff --git a/src/sys/unix/selector/mod.rs b/src/sys/unix/selector/mod.rs index b712b742d..b9be0e1d7 100644 --- a/src/sys/unix/selector/mod.rs +++ b/src/sys/unix/selector/mod.rs @@ -78,13 +78,3 @@ mod kqueue; ), ))] pub(crate) use self::kqueue::{event, Event, Events, Selector}; - -/// Lowest file descriptor used in `Selector::try_clone`. -/// -/// # Notes -/// -/// Usually fds 0, 1 and 2 are standard in, out and error. Some application -/// blindly assume this to be true, which means using any one of those a select -/// could result in some interesting and unexpected errors. Avoid that by using -/// an fd that doesn't have a pre-determined usage. -const LOWEST_FD: libc::c_int = 3; diff --git a/src/sys/unix/selector/poll.rs b/src/sys/unix/selector/poll.rs index bffa87f90..65e176457 100644 --- a/src/sys/unix/selector/poll.rs +++ b/src/sys/unix/selector/poll.rs @@ -11,7 +11,6 @@ use std::sync::{Arc, Condvar, Mutex}; use std::time::Duration; use std::{cmp, fmt, io}; -use crate::sys::unix::selector::LOWEST_FD; use crate::sys::unix::waker::WakerInternal; use crate::{Interest, Token}; @@ -34,9 +33,6 @@ impl Selector { } pub fn try_clone(&self) -> io::Result { - // Just to keep the compiler happy :) - let _ = LOWEST_FD; - let state = self.state.clone(); Ok(Selector { state })