Skip to content

Commit

Permalink
Auto merge of #35048 - tmiasko:monotonic-wait-timeout, r=alexcrichton
Browse files Browse the repository at this point in the history
Use monotonic time in 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.
  • Loading branch information
bors authored Aug 30, 2016
2 parents 4473130 + 59e5e0b commit eac4146
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 9 deletions.
14 changes: 13 additions & 1 deletion src/libstd/sync/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,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
Expand Down Expand Up @@ -138,6 +142,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.
///
Expand All @@ -162,6 +170,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.
///
Expand Down
7 changes: 7 additions & 0 deletions src/libstd/sys/common/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
Expand Down
67 changes: 59 additions & 8 deletions src/libstd/sys/unix/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,43 @@

use cell::UnsafeCell;
use libc;
use ptr;
use sys::mutex::{self, Mutex};
use time::{Instant, Duration};
use time::Duration;

pub struct Condvar { inner: UnsafeCell<libc::pthread_cond_t> }

unsafe impl Send for Condvar {}
unsafe impl Sync for Condvar {}

const TIMESPEC_MAX: libc::timespec = libc::timespec {
tv_sec: <libc::time_t>::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
// initialization of potentially opaque OS data before it landed
Condvar { inner: UnsafeCell::new(libc::PTHREAD_COND_INITIALIZER) }
}

#[cfg(any(target_os = "macos", target_os = "ios", target_os = "android"))]
pub unsafe fn init(&mut self) {}

#[cfg(not(any(target_os = "macos", target_os = "ios", target_os = "android")))]
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());
Expand All @@ -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", target_os = "android")))]
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", target_os = "android"))]
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.
Expand All @@ -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: <libc::time_t>::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),
Expand Down
3 changes: 3 additions & 0 deletions src/libstd/sys/windows/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down

0 comments on commit eac4146

Please sign in to comment.