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

Change poll's timeout from c_int to Option<&Timespec>. #1285

Merged
merged 8 commits into from
Feb 1, 2025
106 changes: 99 additions & 7 deletions src/backend/libc/event/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::event::port::Event;
use crate::event::EventfdFlags;
#[cfg(any(bsd, linux_kernel, target_os = "wasi"))]
use crate::event::FdSetElement;
use crate::event::PollFd;
use crate::event::{PollFd, Timespec};
use crate::io;
#[cfg(solarish)]
use crate::utils::as_mut_ptr;
Expand All @@ -30,7 +30,15 @@ use crate::utils::as_ptr;
all(feature = "alloc", any(linux_kernel, target_os = "redox")),
))]
use core::mem::MaybeUninit;
#[cfg(any(bsd, linux_kernel, target_os = "wasi"))]
#[cfg(any(
bsd,
linux_kernel,
target_os = "fuchsia",
target_os = "haiku",
target_os = "hurd",
target_os = "netbsd",
target_os = "wasi"
))]
use core::ptr::null;
#[cfg(any(bsd, linux_kernel, solarish, target_os = "redox", target_os = "wasi"))]
use core::ptr::null_mut;
Expand Down Expand Up @@ -119,14 +127,98 @@ pub(crate) unsafe fn kevent(
}

#[inline]
pub(crate) fn poll(fds: &mut [PollFd<'_>], timeout: c::c_int) -> io::Result<usize> {
pub(crate) fn poll(fds: &mut [PollFd<'_>], timeout: Option<&Timespec>) -> io::Result<usize> {
let nfds = fds
.len()
.try_into()
.map_err(|_convert_err| io::Errno::INVAL)?;

ret_c_int(unsafe { c::poll(fds.as_mut_ptr().cast(), nfds, timeout) })
.map(|nready| nready as usize)
// If we have `ppoll`, it supports a `timespec` timeout, so use it.
#[cfg(any(
linux_kernel,
freebsdlike,
target_os = "fuchsia",
target_os = "haiku",
target_os = "hurd",
target_os = "netbsd"
))]
{
// If we don't have to fix y2038 on this platform, `Timespec` is
// the same as `c::timespec` and it's easy.
#[cfg(not(fix_y2038))]
let timeout = crate::timespec::option_as_libc_timespec_ptr(timeout);

// If we do have to fix y2038 on this platform, convert to
// `c::timespec`.
#[cfg(fix_y2038)]
let converted_timeout;
#[cfg(fix_y2038)]
let timeout = match timeout {
None => null(),
Some(timeout) => {
converted_timeout = c::timespec {
tv_sec: timeout.tv_sec.try_into().map_err(|_| io::Errno::OVERFLOW)?,
tv_nsec: timeout.tv_nsec as _,
};
&converted_timeout
}
};

#[cfg(not(target_os = "netbsd"))]
{
ret_c_int(unsafe { c::ppoll(fds.as_mut_ptr().cast(), nfds, timeout, null()) })
.map(|nready| nready as usize)
}

// NetBSD 9.x lacks `ppoll`, so use a weak symbol and fall back to
// plain `poll` if needed.
#[cfg(target_os = "netbsd")]
{
weak! {
fn ppoll(
*mut c::pollfd,
c::nfds_t,
*const c::timespec,
*const c::sigset_t
) -> c::c_int
}
if let Some(func) = ppoll.get() {
return ret_c_int(unsafe { func(fds.as_mut_ptr().cast(), nfds, timeout, null()) })
.map(|nready| nready as usize);
}
}
}

// If we don't have `ppoll`, convert the timeout to `c_int` and use `poll`.
#[cfg(not(any(
linux_kernel,
freebsdlike,
target_os = "fuchsia",
target_os = "haiku",
target_os = "hurd"
)))]
{
let timeout = match timeout {
None => -1,
Some(timeout) => {
// Convert from `Timespec` to `c_int` milliseconds.
let secs = timeout.tv_sec;
if secs < 0 {
return Err(io::Errno::INVAL);
}
secs.checked_mul(1000)
.and_then(|millis| {
// Add the nanoseconds, converted to millis, rounding
// up. With Rust 1.73.0 this can use `div_ceil`.
millis.checked_add((i64::from(timeout.tv_nsec) + 999_999) / 1_000_000)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a comment that says the purpose of this is to round up to the nearest millisecond?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet, this looks amazing!

})
.and_then(|millis| c::c_int::try_from(millis).ok())
.ok_or(io::Errno::INVAL)?
}
};
ret_c_int(unsafe { c::poll(fds.as_mut_ptr().cast(), nfds, timeout) })
.map(|nready| nready as usize)
}
}

#[cfg(any(bsd, linux_kernel))]
Expand All @@ -135,7 +227,7 @@ pub(crate) unsafe fn select(
readfds: Option<&mut [FdSetElement]>,
writefds: Option<&mut [FdSetElement]>,
exceptfds: Option<&mut [FdSetElement]>,
timeout: Option<&crate::timespec::Timespec>,
timeout: Option<&Timespec>,
) -> io::Result<i32> {
let len = crate::event::fd_set_num_elements_for_bitvector(nfds);

Expand Down Expand Up @@ -212,7 +304,7 @@ pub(crate) unsafe fn select(
readfds: Option<&mut [FdSetElement]>,
writefds: Option<&mut [FdSetElement]>,
exceptfds: Option<&mut [FdSetElement]>,
timeout: Option<&crate::timespec::Timespec>,
timeout: Option<&Timespec>,
) -> io::Result<i32> {
let len = crate::event::fd_set_num_elements_for_fd_array(nfds as usize);

Expand Down
23 changes: 21 additions & 2 deletions src/backend/libc/event/windows_syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,34 @@

use crate::backend::c;
use crate::backend::conv::ret_c_int;
use crate::event::{FdSetElement, PollFd};
use crate::event::{FdSetElement, PollFd, Timespec};
use crate::io;

pub(crate) fn poll(fds: &mut [PollFd<'_>], timeout: c::c_int) -> io::Result<usize> {
pub(crate) fn poll(fds: &mut [PollFd<'_>], timeout: Option<&Timespec>) -> io::Result<usize> {
let nfds = fds
.len()
.try_into()
.map_err(|_convert_err| io::Errno::INVAL)?;

let timeout = match timeout {
None => -1,
Some(timeout) => {
// Convert from `Timespec` to `c_int` milliseconds.
let secs = timeout.tv_sec;
if secs < 0 {
return Err(io::Errno::INVAL);
}
secs.checked_mul(1000)
.and_then(|millis| {
// Add the nanoseconds, converted to millis, rounding up.
// With Rust 1.73.0 this can use `div_ceil`.
millis.checked_add((i64::from(timeout.tv_nsec) + 999_999) / 1_000_000)
})
.and_then(|millis| c::c_int::try_from(millis).ok())
.ok_or(io::Errno::INVAL)?
}
};

ret_c_int(unsafe { c::poll(fds.as_mut_ptr().cast(), nfds, timeout) })
.map(|nready| nready as usize)
}
Expand Down
1 change: 0 additions & 1 deletion src/backend/linux_raw/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,6 @@ pub(super) fn opt_mut<T: Sized, Num: ArgNumber>(t: Option<&mut T>) -> ArgReg<'_,

/// Convert an optional immutable reference into a `usize` for passing to a
/// syscall.
#[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))]
#[inline]
pub(super) fn opt_ref<T: Sized, Num: ArgNumber>(t: Option<&T>) -> ArgReg<'_, Num> {
// This optimizes into the equivalent of `transmute(t)`, and has the
Expand Down
32 changes: 9 additions & 23 deletions src/backend/linux_raw/event/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,28 @@
//! See the `rustix::backend` module documentation for details.
#![allow(unsafe_code, clippy::undocumented_unsafe_blocks)]

#[cfg(feature = "alloc")]
use crate::backend::c;
use crate::backend::conv::{
by_ref, c_int, c_uint, ret, ret_c_int, ret_error, ret_owned_fd, ret_usize, slice_mut, zero,
by_ref, c_int, c_uint, opt_ref, ret, ret_c_int, ret_error, ret_owned_fd, ret_usize, size_of,
slice_mut, zero,
};
use crate::event::{epoll, EventfdFlags, FdSetElement, PollFd};
use crate::event::{epoll, EventfdFlags, FdSetElement, PollFd, Timespec};
use crate::fd::{BorrowedFd, OwnedFd};
use crate::io;
use crate::utils::as_mut_ptr;
use crate::utils::{as_mut_ptr, option_as_ptr};
#[cfg(feature = "alloc")]
use core::mem::MaybeUninit;
use core::ptr::null_mut;
use linux_raw_sys::general::{EPOLL_CTL_ADD, EPOLL_CTL_DEL, EPOLL_CTL_MOD};
#[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))]
use {
crate::backend::conv::{opt_ref, size_of},
linux_raw_sys::general::{__kernel_timespec, kernel_sigset_t},
};
use linux_raw_sys::general::{kernel_sigset_t, EPOLL_CTL_ADD, EPOLL_CTL_DEL, EPOLL_CTL_MOD};

#[inline]
pub(crate) fn poll(fds: &mut [PollFd<'_>], timeout: c::c_int) -> io::Result<usize> {
pub(crate) fn poll(fds: &mut [PollFd<'_>], timeout: Option<&Timespec>) -> io::Result<usize> {
let (fds_addr_mut, fds_len) = slice_mut(fds);

#[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))]
let timeout = option_as_ptr(timeout);

unsafe {
let timeout = if timeout >= 0 {
Some(__kernel_timespec {
tv_sec: (timeout as i64) / 1000,
tv_nsec: (timeout as i64) % 1000 * 1_000_000,
})
} else {
None
};
ret_usize(syscall!(
__NR_ppoll,
fds_addr_mut,
Expand All @@ -46,10 +36,6 @@ pub(crate) fn poll(fds: &mut [PollFd<'_>], timeout: c::c_int) -> io::Result<usiz
size_of::<kernel_sigset_t, _>()
))
}
#[cfg(not(any(target_arch = "aarch64", target_arch = "riscv64")))]
unsafe {
ret_usize(syscall!(__NR_poll, fds_addr_mut, fds_len, c_int(timeout)))
}
}

pub(crate) unsafe fn select(
Expand Down
1 change: 1 addition & 0 deletions src/event/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub mod port;
#[cfg(any(bsd, linux_kernel, windows, target_os = "wasi"))]
mod select;

pub use crate::timespec::{Nsecs, Secs, Timespec};
#[cfg(any(
linux_kernel,
target_os = "freebsd",
Expand Down
7 changes: 6 additions & 1 deletion src/event/poll.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
use crate::event::Timespec;
use crate::{backend, io};

pub use backend::event::poll_fd::{PollFd, PollFlags};

/// `poll(self.fds, timeout)`—Wait for events on lists of file descriptors.
///
/// Some platforms (those that don't support `ppoll`) don't support timeouts
/// greater than `c_int::MAX` milliseconds; if an unsupported timeout is
/// passed, this function fails with [`io::Errno::INVAL`].
///
/// On macOS, `poll` doesn't work on fds for /dev/tty or /dev/null, however
/// [`select`] is available and does work on these fds.
///
Expand Down Expand Up @@ -32,6 +37,6 @@ pub use backend::event::poll_fd::{PollFd, PollFlags};
/// [DragonFly BSD]: https://man.dragonflybsd.org/?command=poll&section=2
/// [illumos]: https://illumos.org/man/2/poll
#[inline]
pub fn poll(fds: &mut [PollFd<'_>], timeout: i32) -> io::Result<usize> {
pub fn poll(fds: &mut [PollFd<'_>], timeout: Option<&Timespec>) -> io::Result<usize> {
backend::event::syscalls::poll(fds, timeout)
}
3 changes: 1 addition & 2 deletions src/event/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@

#[cfg(any(linux_like, target_os = "wasi"))]
use crate::backend::c;
use crate::event::Timespec;
use crate::fd::RawFd;
use crate::{backend, io};
#[cfg(any(windows, target_os = "wasi"))]
use core::mem::{align_of, size_of};
#[cfg(any(windows, target_os = "wasi"))]
use core::slice;

pub use crate::timespec::{Nsecs, Secs, Timespec};

/// wasi-libc's `fd_set` type. The libc bindings for it have private fields,
/// so we redeclare it for ourselves so that we can access the fields. They're
/// publicly exposed in wasi-libc.
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ mod prctl;
mod signal;
#[cfg(any(
feature = "fs",
feature = "event",
feature = "process",
feature = "runtime",
feature = "thread",
Expand Down
12 changes: 11 additions & 1 deletion src/timespec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
use crate::backend::c;
#[allow(unused)]
use crate::ffi;
#[cfg(not(fix_y2038))]
use core::ptr::null;

/// `struct timespec`
#[derive(Debug, Clone, Copy)]
#[derive(Debug, Clone, Copy, Default)]
#[repr(C)]
pub struct Timespec {
/// Seconds.
Expand Down Expand Up @@ -99,6 +101,14 @@ pub(crate) fn as_libc_timespec_mut_ptr(
timespec.as_mut_ptr().cast::<c::timespec>()
}

#[cfg(not(fix_y2038))]
pub(crate) fn option_as_libc_timespec_ptr(timespec: Option<&Timespec>) -> *const c::timespec {
match timespec {
None => null(),
Some(timespec) => as_libc_timespec_ptr(timespec),
}
}

#[test]
fn test_sizes() {
assert_eq_size!(Secs, u64);
Expand Down
7 changes: 4 additions & 3 deletions tests/event/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use {rustix::event::poll, rustix::io::retry_on_intr};
#[cfg(not(any(windows, target_os = "wasi")))]
#[test]
fn test_poll() {
use rustix::event::Timespec;
use rustix::io::{read, write};
use rustix::pipe::pipe;

Expand All @@ -17,7 +18,7 @@ fn test_poll() {
assert_eq!(poll_fds[0].as_fd().as_raw_fd(), reader.as_fd().as_raw_fd());

// `poll` should say there's nothing ready to be read from the pipe.
let num = retry_on_intr(|| poll(&mut poll_fds, 0)).unwrap();
let num = retry_on_intr(|| poll(&mut poll_fds, Some(&Timespec::default()))).unwrap();
assert_eq!(num, 0);
assert!(poll_fds[0].revents().is_empty());
assert_eq!(poll_fds[0].as_fd().as_raw_fd(), reader.as_fd().as_raw_fd());
Expand All @@ -26,7 +27,7 @@ fn test_poll() {
assert_eq!(retry_on_intr(|| write(&writer, b"a")).unwrap(), 1);

// `poll` should now say there's data to be read.
let num = retry_on_intr(|| poll(&mut poll_fds, -1)).unwrap();
let num = retry_on_intr(|| poll(&mut poll_fds, None)).unwrap();
assert_eq!(num, 1);
assert_eq!(poll_fds[0].revents(), PollFlags::IN);
assert_eq!(poll_fds[0].as_fd().as_raw_fd(), reader.as_fd().as_raw_fd());
Expand All @@ -43,7 +44,7 @@ fn test_poll() {
assert_eq!(poll_fds[0].as_fd().as_raw_fd(), reader.as_fd().as_raw_fd());

// Poll should now say there's no more data to be read.
let num = retry_on_intr(|| poll(&mut poll_fds, 0)).unwrap();
let num = retry_on_intr(|| poll(&mut poll_fds, Some(&Timespec::default()))).unwrap();
assert_eq!(num, 0);
assert!(poll_fds[0].revents().is_empty());
assert_eq!(poll_fds[0].as_fd().as_raw_fd(), reader.as_fd().as_raw_fd());
Expand Down
Loading