Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement I/O safety traits #1745

Merged
merged 4 commits into from
Jun 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Use OwnedFd in epoll and kqueue selectors
  • Loading branch information
Thomasdezeeuw committed Jun 9, 2024
commit 5ca5e2537f82010654496138432aa6c2e1462ece
1 change: 1 addition & 0 deletions src/sys/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
31 changes: 14 additions & 17 deletions src/sys/unix/selector/epoll.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<Selector> {
// 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<Selector> {
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,
Expand All @@ -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,
Expand All @@ -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<()> {
Expand All @@ -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(|_| ())
}
}

Expand All @@ -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()
}
}

Expand Down
41 changes: 17 additions & 24 deletions src/sys/unix/selector/kqueue.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<Selector> {
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<Selector> {
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,
Expand All @@ -106,7 +105,7 @@ impl Selector {

events.clear();
syscall!(kevent(
self.kq,
self.kq.as_raw_fd(),
ptr::null(),
0,
events.as_mut_ptr(),
Expand Down Expand Up @@ -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<()> {
Expand Down Expand Up @@ -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],
)
Expand All @@ -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`.
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
}
}

Expand Down
10 changes: 0 additions & 10 deletions src/sys/unix/selector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
4 changes: 0 additions & 4 deletions src/sys/unix/selector/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -34,9 +33,6 @@ impl Selector {
}

pub fn try_clone(&self) -> io::Result<Selector> {
// Just to keep the compiler happy :)
let _ = LOWEST_FD;

let state = self.state.clone();

Ok(Selector { state })
Expand Down