Skip to content

Commit 416b4c3

Browse files
authored
Handle Duration overflow gracefully in kevent. (#1427)
Add a `kevent_timespec` function, which is similar to `kevent`, but which takes a `Timespec` rather than a `Duration`, so that it can behave consistently with the other functions in `rustix::event`. This also more closely reflects how the underlying `kevent` function takes a `timespec` argument. And, change the existing `kevent` to treat overflow in its `Duration` argument by saturating to an effectively infinite timeout. This supports users that use `Duration::MAX` to indicate an effectively infinite timeout.
1 parent bd5427c commit 416b4c3

File tree

2 files changed

+60
-19
lines changed

2 files changed

+60
-19
lines changed

src/backend/libc/event/syscalls.rs

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ use crate::backend::conv::ret;
66
use crate::backend::conv::ret_c_int;
77
#[cfg(any(linux_kernel, target_os = "illumos", target_os = "redox"))]
88
use crate::backend::conv::ret_u32;
9+
#[cfg(bsd)]
10+
use crate::event::kqueue::Event;
911
#[cfg(solarish)]
1012
use crate::event::port::Event;
1113
#[cfg(any(
@@ -47,8 +49,6 @@ use {crate::backend::conv::borrowed_fd, crate::fd::BorrowedFd};
4749
target_os = "redox"
4850
))]
4951
use {crate::backend::conv::ret_owned_fd, crate::fd::OwnedFd};
50-
#[cfg(bsd)]
51-
use {crate::event::kqueue::Event, crate::utils::as_ptr};
5252

5353
#[cfg(any(
5454
linux_kernel,
@@ -96,8 +96,28 @@ pub(crate) unsafe fn kevent(
9696
kq: BorrowedFd<'_>,
9797
changelist: &[Event],
9898
eventlist: (*mut Event, usize),
99-
timeout: Option<&c::timespec>,
99+
timeout: Option<&Timespec>,
100100
) -> io::Result<c::c_int> {
101+
// If we don't have to fix y2038 on this platform, `Timespec` is the same
102+
// as `c::timespec` and it's easy.
103+
#[cfg(not(fix_y2038))]
104+
let timeout = crate::timespec::option_as_libc_timespec_ptr(timeout);
105+
106+
// If we do have to fix y2038 on this platform, convert to `c::timespec`.
107+
#[cfg(fix_y2038)]
108+
let converted_timeout;
109+
#[cfg(fix_y2038)]
110+
let timeout = match timeout {
111+
None => null(),
112+
Some(timeout) => {
113+
converted_timeout = c::timespec {
114+
tv_sec: timeout.tv_sec.try_into().map_err(|_| io::Errno::OVERFLOW)?,
115+
tv_nsec: timeout.tv_nsec as _,
116+
};
117+
&converted_timeout
118+
}
119+
};
120+
101121
ret_c_int(c::kevent(
102122
borrowed_fd(kq),
103123
changelist.as_ptr().cast(),
@@ -107,7 +127,7 @@ pub(crate) unsafe fn kevent(
107127
.map_err(|_| io::Errno::OVERFLOW)?,
108128
eventlist.0.cast(),
109129
eventlist.1.try_into().map_err(|_| io::Errno::OVERFLOW)?,
110-
timeout.map_or(null(), as_ptr),
130+
timeout,
111131
))
112132
}
113133

src/event/kqueue.rs

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::buffer::Buffer;
44
use crate::fd::{AsFd, OwnedFd, RawFd};
55
use crate::pid::Pid;
66
use crate::signal::Signal;
7+
use crate::timespec::Timespec;
78
use crate::{backend, io};
89

910
use backend::c::{self, intptr_t, kevent as kevent_t, uintptr_t};
@@ -418,28 +419,48 @@ pub fn kqueue() -> io::Result<OwnedFd> {
418419
/// [OpenBSD]: https://man.openbsd.org/kevent.2
419420
/// [NetBSD]: https://man.netbsd.org/kevent.2
420421
/// [DragonFly BSD]: https://man.dragonflybsd.org/?command=kevent&section=2
421-
pub unsafe fn kevent<Fd: AsFd, Buf: Buffer<Event>>(
422+
pub unsafe fn kevent_timespec<Fd: AsFd, Buf: Buffer<Event>>(
422423
kqueue: Fd,
423424
changelist: &[Event],
424425
mut eventlist: Buf,
426+
timeout: Option<&Timespec>,
427+
) -> io::Result<Buf::Output> {
428+
// Populate the event list with events.
429+
let len = syscalls::kevent(kqueue.as_fd(), changelist, eventlist.parts_mut(), timeout)
430+
.map(|res| res as _)?;
431+
432+
Ok(eventlist.assume_init(len))
433+
}
434+
435+
/// `kevent(kqueue, changelist, eventlist, timeout)`—Wait for events on a
436+
/// `kqueue`.
437+
///
438+
/// This is a wrapper around [`kevent_timespec`] which takes a `Duration`
439+
/// instead of a `Timespec` for the timemout value. `Timespec` has a signed
440+
/// `i64` seconds field; if converting `Duration` to `Timespec` overflows,
441+
/// `None` is passed as the timeout instead, such such a large timeout would
442+
/// be effectively infinite in practice.
443+
///
444+
/// # Safety
445+
///
446+
/// The file descriptors referred to by the `Event` structs must be valid for
447+
/// the lifetime of the `kqueue` file descriptor.
448+
pub unsafe fn kevent<Fd: AsFd, Buf: Buffer<Event>>(
449+
kqueue: Fd,
450+
changelist: &[Event],
451+
eventlist: Buf,
425452
timeout: Option<Duration>,
426453
) -> io::Result<Buf::Output> {
427454
let timeout = match timeout {
428-
Some(timeout) => Some(backend::c::timespec {
429-
tv_sec: timeout.as_secs().try_into().map_err(|_| io::Errno::INVAL)?,
430-
tv_nsec: timeout.subsec_nanos() as _,
431-
}),
455+
Some(timeout) => match timeout.as_secs().try_into() {
456+
Ok(tv_sec) => Some(Timespec {
457+
tv_sec,
458+
tv_nsec: timeout.subsec_nanos() as _,
459+
}),
460+
Err(_) => None,
461+
},
432462
None => None,
433463
};
434464

435-
// Populate the event list with events.
436-
let len = syscalls::kevent(
437-
kqueue.as_fd(),
438-
changelist,
439-
eventlist.parts_mut(),
440-
timeout.as_ref(),
441-
)
442-
.map(|res| res as _)?;
443-
444-
Ok(eventlist.assume_init(len))
465+
kevent_timespec(kqueue, changelist, eventlist, timeout.as_ref())
445466
}

0 commit comments

Comments
 (0)