From 8ec47261e193b1f983b898a0d985b85e7fcb0668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Tue, 26 Jul 2016 15:18:44 +0200 Subject: [PATCH 1/2] Use monotonic time with condition variables. Configure condition variables to use monotonic time using pthread_condattr_setclock on systems where this is possible. This fixes the issue when thread waiting on condition variable is woken up too late when system time is moved backwards. --- src/libstd/sync/condvar.rs | 6 ++- src/libstd/sys/common/condvar.rs | 7 ++++ src/libstd/sys/unix/condvar.rs | 67 +++++++++++++++++++++++++++---- src/libstd/sys/windows/condvar.rs | 3 ++ 4 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/libstd/sync/condvar.rs b/src/libstd/sync/condvar.rs index 3c52ebc72f2cb..ca1a9905ebe67 100644 --- a/src/libstd/sync/condvar.rs +++ b/src/libstd/sync/condvar.rs @@ -82,10 +82,14 @@ impl Condvar { /// notified. #[stable(feature = "rust1", since = "1.0.0")] pub fn new() -> Condvar { - Condvar { + let mut c = Condvar { inner: box sys::Condvar::new(), mutex: AtomicUsize::new(0), + }; + unsafe { + c.inner.init(); } + c } /// Blocks the current thread until this condition variable receives a diff --git a/src/libstd/sys/common/condvar.rs b/src/libstd/sys/common/condvar.rs index 33734a88cf32b..b6f29dd5fc3d3 100644 --- a/src/libstd/sys/common/condvar.rs +++ b/src/libstd/sys/common/condvar.rs @@ -27,6 +27,13 @@ impl Condvar { /// first used with any of the functions below. pub const fn new() -> Condvar { Condvar(imp::Condvar::new()) } + /// Prepares the condition variable for use. + /// + /// This should be called once the condition variable is at a stable memory + /// address. + #[inline] + pub unsafe fn init(&mut self) { self.0.init() } + /// Signals one waiter on this condition variable to wake up. #[inline] pub unsafe fn notify_one(&self) { self.0.notify_one() } diff --git a/src/libstd/sys/unix/condvar.rs b/src/libstd/sys/unix/condvar.rs index 2e1c1900b46b3..725a071a4f9fe 100644 --- a/src/libstd/sys/unix/condvar.rs +++ b/src/libstd/sys/unix/condvar.rs @@ -10,15 +10,19 @@ use cell::UnsafeCell; use libc; -use ptr; use sys::mutex::{self, Mutex}; -use time::{Instant, Duration}; +use time::Duration; pub struct Condvar { inner: UnsafeCell } unsafe impl Send for Condvar {} unsafe impl Sync for Condvar {} +const TIMESPEC_MAX: libc::timespec = libc::timespec { + tv_sec: ::max_value(), + tv_nsec: 1_000_000_000 - 1, +}; + impl Condvar { pub const fn new() -> Condvar { // Might be moved and address is changing it is better to avoid @@ -26,6 +30,23 @@ impl Condvar { Condvar { inner: UnsafeCell::new(libc::PTHREAD_COND_INITIALIZER) } } + #[cfg(any(target_os = "macos", target_os = "ios"))] + pub unsafe fn init(&mut self) {} + + #[cfg(not(any(target_os = "macos", target_os = "ios")))] + pub unsafe fn init(&mut self) { + use mem; + let mut attr: libc::pthread_condattr_t = mem::uninitialized(); + let r = libc::pthread_condattr_init(&mut attr); + assert_eq!(r, 0); + let r = libc::pthread_condattr_setclock(&mut attr, libc::CLOCK_MONOTONIC); + assert_eq!(r, 0); + let r = libc::pthread_cond_init(self.inner.get(), &attr); + assert_eq!(r, 0); + let r = libc::pthread_condattr_destroy(&mut attr); + assert_eq!(r, 0); + } + #[inline] pub unsafe fn notify_one(&self) { let r = libc::pthread_cond_signal(self.inner.get()); @@ -44,10 +65,45 @@ impl Condvar { debug_assert_eq!(r, 0); } + // This implementation is used on systems that support pthread_condattr_setclock + // where we configure condition variable to use monotonic clock (instead of + // default system clock). This approach avoids all problems that result + // from changes made to the system time. + #[cfg(not(any(target_os = "macos", target_os = "ios")))] + pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool { + use mem; + + 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() as libc::c_long + now.tv_nsec as libc::c_long; + // FIXME: Casting u64 into time_t could truncate the value. + let sec = (dur.as_secs() as libc::time_t) + .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 } + }).unwrap_or(TIMESPEC_MAX); + + let r = libc::pthread_cond_timedwait(self.inner.get(), mutex::raw(mutex), + &timeout); + assert!(r == libc::ETIMEDOUT || r == 0); + r == 0 + } + + // This implementation is modeled after libcxx's condition_variable // https://github.com/llvm-mirror/libcxx/blob/release_35/src/condition_variable.cpp#L46 // https://github.com/llvm-mirror/libcxx/blob/release_35/include/__mutex_base#L367 + #[cfg(any(target_os = "macos", target_os = "ios"))] pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool { + use ptr; + use time::Instant; + // 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. @@ -66,12 +122,7 @@ impl Condvar { s.checked_add(seconds) }).map(|s| { libc::timespec { tv_sec: s, tv_nsec: nsec } - }).unwrap_or_else(|| { - libc::timespec { - tv_sec: ::max_value(), - tv_nsec: 1_000_000_000 - 1, - } - }); + }).unwrap_or(TIMESPEC_MAX); // And wait! let r = libc::pthread_cond_timedwait(self.inner.get(), mutex::raw(mutex), diff --git a/src/libstd/sys/windows/condvar.rs b/src/libstd/sys/windows/condvar.rs index 8075374d42bdd..d708b327c55cb 100644 --- a/src/libstd/sys/windows/condvar.rs +++ b/src/libstd/sys/windows/condvar.rs @@ -24,6 +24,9 @@ impl Condvar { Condvar { inner: UnsafeCell::new(c::CONDITION_VARIABLE_INIT) } } + #[inline] + pub unsafe fn init(&mut self) {} + #[inline] pub unsafe fn wait(&self, mutex: &Mutex) { let r = c::SleepConditionVariableSRW(self.inner.get(), From 8dae1b662588ed18017a50c2cbc5c6b2880ae27c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 19 Aug 2016 14:00:20 +0200 Subject: [PATCH 2/2] Document that Condvar makes the best effort to use monotonic clock. --- src/libstd/sync/condvar.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libstd/sync/condvar.rs b/src/libstd/sync/condvar.rs index ca1a9905ebe67..5ee42a09e12ae 100644 --- a/src/libstd/sync/condvar.rs +++ b/src/libstd/sync/condvar.rs @@ -144,6 +144,10 @@ impl Condvar { /// differences that may not cause the maximum amount of time /// waited to be precisely `ms`. /// + /// Note that the best effort is made to ensure that the time waited is + /// measured with a monotonic clock, and not affected by the changes made to + /// the system time. + /// /// The returned boolean is `false` only if the timeout is known /// to have elapsed. /// @@ -168,6 +172,10 @@ impl Condvar { /// preemption or platform differences that may not cause the maximum /// amount of time waited to be precisely `dur`. /// + /// Note that the best effort is made to ensure that the time waited is + /// measured with a monotonic clock, and not affected by the changes made to + /// the system time. + /// /// The returned `WaitTimeoutResult` value indicates if the timeout is /// known to have elapsed. ///