Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use monotonic time in condition variables. #35048

Merged
merged 3 commits into from Aug 31, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/libstd/sync/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -140,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.
///
Expand All @@ -164,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.
///
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems very similar to the implementation below, could the two be unified?

}


// 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