Skip to content

Commit

Permalink
Rollup merge of #103146 - joboet:cleanup_pthread_condvar, r=Mark-Simu…
Browse files Browse the repository at this point in the history 
…lacrum

Cleanup timeouts in pthread condvar
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Dec 10, 2022
2 parents c6fcdb6 + da0a542 commit 7f4e7c1
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 66 deletions.
93 changes: 30 additions & 63 deletions library/std/src/sys/unix/locks/pthread_condvar.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::cell::UnsafeCell;
use crate::ptr;
use crate::sync::atomic::{AtomicPtr, Ordering::Relaxed};
use crate::sys::locks::{pthread_mutex, Mutex};
use crate::sys::time::TIMESPEC_MAX;
use crate::sys_common::lazy_box::{LazyBox, LazyInit};
use crate::time::Duration;

Expand All @@ -12,13 +13,6 @@ pub struct Condvar {
mutex: AtomicPtr<libc::pthread_mutex_t>,
}

const TIMESPEC_MAX: libc::timespec =
libc::timespec { tv_sec: <libc::time_t>::MAX, tv_nsec: 1_000_000_000 - 1 };

fn saturating_cast_to_time_t(value: u64) -> libc::time_t {
if value > <libc::time_t>::MAX as u64 { <libc::time_t>::MAX } else { value as libc::time_t }
}

#[inline]
fn raw(c: &Condvar) -> *mut libc::pthread_cond_t {
c.inner.0.get()
Expand Down Expand Up @@ -133,26 +127,15 @@ impl Condvar {
target_os = "horizon"
)))]
pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
use crate::mem;
use crate::sys::time::Timespec;

let mutex = pthread_mutex::raw(mutex);
self.verify(mutex);

let mut now: libc::timespec = mem::zeroed();
let r = libc::clock_gettime(libc::CLOCK_MONOTONIC, &mut now);
assert_eq!(r, 0);

// Nanosecond calculations can't overflow because both values are below 1e9.
let nsec = dur.subsec_nanos() + now.tv_nsec as u32;

let sec = saturating_cast_to_time_t(dur.as_secs())
.checked_add((nsec / 1_000_000_000) as libc::time_t)
.and_then(|s| s.checked_add(now.tv_sec));
let nsec = nsec % 1_000_000_000;

let timeout =
sec.map(|s| libc::timespec { tv_sec: s, tv_nsec: nsec as _ }).unwrap_or(TIMESPEC_MAX);

let timeout = Timespec::now(libc::CLOCK_MONOTONIC)
.checked_add_duration(&dur)
.and_then(|t| t.to_timespec())
.unwrap_or(TIMESPEC_MAX);
let r = libc::pthread_cond_timedwait(raw(self), mutex, &timeout);
assert!(r == libc::ETIMEDOUT || r == 0);
r == 0
Expand All @@ -169,57 +152,41 @@ impl Condvar {
target_os = "espidf",
target_os = "horizon"
))]
pub unsafe fn wait_timeout(&self, mutex: &Mutex, mut dur: Duration) -> bool {
pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
use crate::sys::time::SystemTime;
use crate::time::Instant;

let mutex = pthread_mutex::raw(mutex);
self.verify(mutex);

// 1000 years
let max_dur = Duration::from_secs(1000 * 365 * 86400);

if dur > max_dur {
// OSX implementation of `pthread_cond_timedwait` is buggy
// with super long durations. When duration is greater than
// 0x100_0000_0000_0000 seconds, `pthread_cond_timedwait`
// in macOS Sierra return error 316.
//
// This program demonstrates the issue:
// https://gist.github.com/stepancheg/198db4623a20aad2ad7cddb8fda4a63c
//
// To work around this issue, and possible bugs of other OSes, timeout
// is clamped to 1000 years, which is allowable per the API of `wait_timeout`
// because of spurious wakeups.

dur = max_dur;
}

// First, figure out what time it currently is, in both system and
// stable time. pthread_cond_timedwait uses system time, but we want to
// report timeout based on stable time.
let mut sys_now = libc::timeval { tv_sec: 0, tv_usec: 0 };
let stable_now = Instant::now();
let r = libc::gettimeofday(&mut sys_now, ptr::null_mut());
assert_eq!(r, 0, "unexpected error: {:?}", crate::io::Error::last_os_error());

let nsec = dur.subsec_nanos() as libc::c_long + (sys_now.tv_usec * 1000) as libc::c_long;
let extra = (nsec / 1_000_000_000) as libc::time_t;
let nsec = nsec % 1_000_000_000;
let seconds = saturating_cast_to_time_t(dur.as_secs());

let timeout = sys_now
.tv_sec
.checked_add(extra)
.and_then(|s| s.checked_add(seconds))
.map(|s| libc::timespec { tv_sec: s, tv_nsec: nsec })
// OSX implementation of `pthread_cond_timedwait` is buggy
// with super long durations. When duration is greater than
// 0x100_0000_0000_0000 seconds, `pthread_cond_timedwait`
// in macOS Sierra returns error 316.
//
// This program demonstrates the issue:
// https://gist.github.com/stepancheg/198db4623a20aad2ad7cddb8fda4a63c
//
// To work around this issue, and possible bugs of other OSes, timeout
// is clamped to 1000 years, which is allowable per the API of `wait_timeout`
// because of spurious wakeups.
let dur = Duration::min(dur, Duration::from_secs(1000 * 365 * 86400));

// pthread_cond_timedwait uses system time, but we want to report timeout
// based on stable time.
let now = Instant::now();

let timeout = SystemTime::now()
.t
.checked_add_duration(&dur)
.and_then(|t| t.to_timespec())
.unwrap_or(TIMESPEC_MAX);

// And wait!
let r = libc::pthread_cond_timedwait(raw(self), mutex, &timeout);
debug_assert!(r == libc::ETIMEDOUT || r == 0);

// ETIMEDOUT is not a totally reliable method of determining timeout due
// to clock shifts, so do the check ourselves
stable_now.elapsed() < dur
now.elapsed() < dur
}
}
4 changes: 1 addition & 3 deletions library/std/src/sys/unix/thread_parker/pthread.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::pin::Pin;
use crate::ptr::addr_of_mut;
use crate::sync::atomic::AtomicUsize;
use crate::sync::atomic::Ordering::SeqCst;
use crate::sys::time::TIMESPEC_MAX;
use crate::time::Duration;

const EMPTY: usize = 0;
Expand All @@ -32,9 +33,6 @@ unsafe fn wait(cond: *mut libc::pthread_cond_t, lock: *mut libc::pthread_mutex_t
debug_assert_eq!(r, 0);
}

const TIMESPEC_MAX: libc::timespec =
libc::timespec { tv_sec: <libc::time_t>::MAX, tv_nsec: 1_000_000_000 - 1 };

unsafe fn wait_timeout(
cond: *mut libc::pthread_cond_t,
lock: *mut libc::pthread_mutex_t,
Expand Down
3 changes: 3 additions & 0 deletions library/std/src/sys/unix/time.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ pub use self::inner::Instant;

const NSEC_PER_SEC: u64 = 1_000_000_000;
pub const UNIX_EPOCH: SystemTime = SystemTime { t: Timespec::zero() };
#[allow(dead_code)] // Used for pthread condvar timeouts
pub const TIMESPEC_MAX: libc::timespec =
libc::timespec { tv_sec: <libc::time_t>::MAX, tv_nsec: 1_000_000_000 - 1 };

#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[repr(transparent)]
Expand Down

0 comments on commit 7f4e7c1

Please sign in to comment.