Skip to content

Commit

Permalink
Clamp long timeouts for POSIX
Browse files Browse the repository at this point in the history
The previous timeout calculation overflowed at a cast and resulted in
negative timeouts. These in turn resulted in errors from ppoll.
  • Loading branch information
sirhcel committed Aug 2, 2024
1 parent abefa01 commit 129b4d1
Showing 1 changed file with 63 additions and 31 deletions.
94 changes: 63 additions & 31 deletions src/posix/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ use std::os::unix::io::RawFd;
use std::slice;
use std::time::Duration;

use nix::libc::c_int;
use nix::poll::{PollFd, PollFlags};
#[cfg(target_os = "linux")]
use nix::sys::signal::SigSet;
#[cfg(target_os = "linux")]
use nix::sys::time::{TimeSpec, TimeValLike};
#[cfg(any(target_os = "linux", test))]
use nix::sys::time::TimeSpec;

pub fn wait_read_fd(fd: RawFd, timeout: Duration) -> io::Result<()> {
wait_fd(fd, PollFlags::POLLIN, timeout)
Expand All @@ -24,22 +25,12 @@ fn wait_fd(fd: RawFd, events: PollFlags, timeout: Duration) -> io::Result<()> {

let mut fd = PollFd::new(fd, events);

let milliseconds = milliseconds_i64(timeout);
#[cfg(target_os = "linux")]
let wait_res = {
let timespec = TimeSpec::milliseconds(milliseconds);
nix::poll::ppoll(
slice::from_mut(&mut fd),
Some(timespec),
Some(SigSet::empty()),
)
};
#[cfg(not(target_os = "linux"))]
let wait_res = nix::poll::poll(slice::from_mut(&mut fd), milliseconds as nix::libc::c_int);

let wait = match wait_res {
let wait = match poll_clamped(&mut fd, timeout) {
Ok(r) => r,
Err(e) => return Err(io::Error::from(crate::Error::from(e))),
Err(e) => {
dbg!(e);
return Err(io::Error::from(crate::Error::from(e)));
}
};
// All errors generated by poll or ppoll are already caught by the nix wrapper around libc, so
// here we only need to check if there's at least 1 event
Expand All @@ -63,16 +54,55 @@ fn wait_fd(fd: RawFd, events: PollFlags, timeout: Duration) -> io::Result<()> {
Err(io::Error::new(io::ErrorKind::Other, EIO.desc()))
}

fn milliseconds_i64(duration: Duration) -> i64 {
duration.as_secs() as i64 * 1000 + i64::from(duration.subsec_nanos()) / 1_000_000
/// Poll with a duration clamped to the maximum value representable by the `TimeSpec` used by
/// `ppoll`.
#[cfg(target_os = "linux")]
fn poll_clamped(fd: &mut PollFd, timeout: Duration) -> nix::Result<c_int> {
let spec = clamped_time_spec(timeout);
nix::poll::ppoll(slice::from_mut(fd), Some(spec), Some(SigSet::empty()))
}

#[cfg(any(target_os = "linux", test))]
fn clamped_time_spec(duration: Duration) -> TimeSpec {
use nix::libc::c_long;
use nix::sys::time::time_t;

// We need to clamp manually as TimeSpec::from_duration translates durations with more than
// i64::MAX seconds to negative timespans. This happens due to casting to i64 and is still the
// case as of nix 0.29.
let secs_limit = time_t::MAX as u64;
let secs = duration.as_secs();
if secs <= secs_limit {
TimeSpec::new(secs as time_t, duration.subsec_nanos() as c_long)
} else {
TimeSpec::new(time_t::MAX, 999_999_999)
}
}

// Poll with a duration clamped to the maximum millisecond value representable by the `c_int` used
// by `poll`.
#[cfg(not(target_os = "linux"))]
fn poll_clamped(fd: &mut PollFd, timeout: Duration) -> nix::Result<c_int> {
let millis = clamped_millis_c_int(timeout);
nix::poll::poll(slice::from_mut(fd), millis)
}

#[cfg(any(not(target_os = "linux"), test))]
fn clamped_millis_c_int(duration: Duration) -> c_int {
let secs_limit = (c_int::MAX as u64) / 1000;
let secs = duration.as_secs();

if secs <= secs_limit {
secs as c_int * 1000 + duration.subsec_millis() as c_int
} else {
c_int::MAX
}
}

#[cfg(test)]
mod test {
use super::*;

use nix::libc::c_int;

// TODO: Harmonize with corresponding tests for Windows.
fn monotonicity_test_durations() -> Vec<Duration> {
vec![
Expand All @@ -97,11 +127,11 @@ mod test {
}

#[test]
fn milliseconds_i64_as_c_int_is_monotonic() {
let mut last = milliseconds_i64(Duration::ZERO) as c_int;
fn clamped_millis_c_int_is_monotonic() {
let mut last = clamped_millis_c_int(Duration::ZERO);

for (i, d) in monotonicity_test_durations().iter().enumerate() {
let next = milliseconds_i64(*d) as c_int;
let next = clamped_millis_c_int(*d);
dbg!((i, d));
assert!(
next >= last,
Expand All @@ -112,16 +142,16 @@ mod test {
}

#[test]
fn milliseconds_i64_as_c_int_zero_is_zero() {
assert_eq!(0, milliseconds_i64(Duration::ZERO) as c_int);
fn clamped_millis_c_int_zero_is_zero() {
assert_eq!(0, clamped_millis_c_int(Duration::ZERO));
}

#[test]
fn milliseconds_i64_is_monotonic() {
let mut last = milliseconds_i64(Duration::ZERO);
fn clamped_time_spec_is_monotonic() {
let mut last = clamped_time_spec(Duration::ZERO);

for (i, d) in monotonicity_test_durations().iter().enumerate() {
let next = milliseconds_i64(*d);
let next = clamped_time_spec(*d);
dbg!((i, d));
assert!(
next >= last,
Expand All @@ -132,7 +162,9 @@ mod test {
}

#[test]
fn milliseconds_i64_zero_is_zero() {
assert_eq!(0, milliseconds_i64(Duration::ZERO));
fn clamped_time_spec_zero_is_zero() {
let spec = clamped_time_spec(Duration::ZERO);
assert_eq!(0, spec.tv_sec());
assert_eq!(0, spec.tv_nsec());
}
}

0 comments on commit 129b4d1

Please sign in to comment.