Skip to content

Commit 388a060

Browse files
authored
Auto merge of #35048 - tmiasko:monotonic-wait-timeout, r=alexcrichton
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.
2 parents 599f1b9 + 8dae1b6 commit 388a060

File tree

4 files changed

+82
-9
lines changed

4 files changed

+82
-9
lines changed

src/libstd/sync/condvar.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,14 @@ impl Condvar {
8282
/// notified.
8383
#[stable(feature = "rust1", since = "1.0.0")]
8484
pub fn new() -> Condvar {
85-
Condvar {
85+
let mut c = Condvar {
8686
inner: box sys::Condvar::new(),
8787
mutex: AtomicUsize::new(0),
88+
};
89+
unsafe {
90+
c.inner.init();
8891
}
92+
c
8993
}
9094

9195
/// Blocks the current thread until this condition variable receives a
@@ -140,6 +144,10 @@ impl Condvar {
140144
/// differences that may not cause the maximum amount of time
141145
/// waited to be precisely `ms`.
142146
///
147+
/// Note that the best effort is made to ensure that the time waited is
148+
/// measured with a monotonic clock, and not affected by the changes made to
149+
/// the system time.
150+
///
143151
/// The returned boolean is `false` only if the timeout is known
144152
/// to have elapsed.
145153
///
@@ -164,6 +172,10 @@ impl Condvar {
164172
/// preemption or platform differences that may not cause the maximum
165173
/// amount of time waited to be precisely `dur`.
166174
///
175+
/// Note that the best effort is made to ensure that the time waited is
176+
/// measured with a monotonic clock, and not affected by the changes made to
177+
/// the system time.
178+
///
167179
/// The returned `WaitTimeoutResult` value indicates if the timeout is
168180
/// known to have elapsed.
169181
///

src/libstd/sys/common/condvar.rs

+7
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ impl Condvar {
2727
/// first used with any of the functions below.
2828
pub const fn new() -> Condvar { Condvar(imp::Condvar::new()) }
2929

30+
/// Prepares the condition variable for use.
31+
///
32+
/// This should be called once the condition variable is at a stable memory
33+
/// address.
34+
#[inline]
35+
pub unsafe fn init(&mut self) { self.0.init() }
36+
3037
/// Signals one waiter on this condition variable to wake up.
3138
#[inline]
3239
pub unsafe fn notify_one(&self) { self.0.notify_one() }

src/libstd/sys/unix/condvar.rs

+59-8
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,43 @@
1010

1111
use cell::UnsafeCell;
1212
use libc;
13-
use ptr;
1413
use sys::mutex::{self, Mutex};
15-
use time::{Instant, Duration};
14+
use time::Duration;
1615

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

1918
unsafe impl Send for Condvar {}
2019
unsafe impl Sync for Condvar {}
2120

21+
const TIMESPEC_MAX: libc::timespec = libc::timespec {
22+
tv_sec: <libc::time_t>::max_value(),
23+
tv_nsec: 1_000_000_000 - 1,
24+
};
25+
2226
impl Condvar {
2327
pub const fn new() -> Condvar {
2428
// Might be moved and address is changing it is better to avoid
2529
// initialization of potentially opaque OS data before it landed
2630
Condvar { inner: UnsafeCell::new(libc::PTHREAD_COND_INITIALIZER) }
2731
}
2832

33+
#[cfg(any(target_os = "macos", target_os = "ios"))]
34+
pub unsafe fn init(&mut self) {}
35+
36+
#[cfg(not(any(target_os = "macos", target_os = "ios")))]
37+
pub unsafe fn init(&mut self) {
38+
use mem;
39+
let mut attr: libc::pthread_condattr_t = mem::uninitialized();
40+
let r = libc::pthread_condattr_init(&mut attr);
41+
assert_eq!(r, 0);
42+
let r = libc::pthread_condattr_setclock(&mut attr, libc::CLOCK_MONOTONIC);
43+
assert_eq!(r, 0);
44+
let r = libc::pthread_cond_init(self.inner.get(), &attr);
45+
assert_eq!(r, 0);
46+
let r = libc::pthread_condattr_destroy(&mut attr);
47+
assert_eq!(r, 0);
48+
}
49+
2950
#[inline]
3051
pub unsafe fn notify_one(&self) {
3152
let r = libc::pthread_cond_signal(self.inner.get());
@@ -44,10 +65,45 @@ impl Condvar {
4465
debug_assert_eq!(r, 0);
4566
}
4667

68+
// This implementation is used on systems that support pthread_condattr_setclock
69+
// where we configure condition variable to use monotonic clock (instead of
70+
// default system clock). This approach avoids all problems that result
71+
// from changes made to the system time.
72+
#[cfg(not(any(target_os = "macos", target_os = "ios")))]
73+
pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
74+
use mem;
75+
76+
let mut now: libc::timespec = mem::zeroed();
77+
let r = libc::clock_gettime(libc::CLOCK_MONOTONIC, &mut now);
78+
assert_eq!(r, 0);
79+
80+
// Nanosecond calculations can't overflow because both values are below 1e9.
81+
let nsec = dur.subsec_nanos() as libc::c_long + now.tv_nsec as libc::c_long;
82+
// FIXME: Casting u64 into time_t could truncate the value.
83+
let sec = (dur.as_secs() as libc::time_t)
84+
.checked_add((nsec / 1_000_000_000) as libc::time_t)
85+
.and_then(|s| s.checked_add(now.tv_sec));
86+
let nsec = nsec % 1_000_000_000;
87+
88+
let timeout = sec.map(|s| {
89+
libc::timespec { tv_sec: s, tv_nsec: nsec }
90+
}).unwrap_or(TIMESPEC_MAX);
91+
92+
let r = libc::pthread_cond_timedwait(self.inner.get(), mutex::raw(mutex),
93+
&timeout);
94+
assert!(r == libc::ETIMEDOUT || r == 0);
95+
r == 0
96+
}
97+
98+
4799
// This implementation is modeled after libcxx's condition_variable
48100
// https://github.com/llvm-mirror/libcxx/blob/release_35/src/condition_variable.cpp#L46
49101
// https://github.com/llvm-mirror/libcxx/blob/release_35/include/__mutex_base#L367
102+
#[cfg(any(target_os = "macos", target_os = "ios"))]
50103
pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
104+
use ptr;
105+
use time::Instant;
106+
51107
// First, figure out what time it currently is, in both system and
52108
// stable time. pthread_cond_timedwait uses system time, but we want to
53109
// report timeout based on stable time.
@@ -66,12 +122,7 @@ impl Condvar {
66122
s.checked_add(seconds)
67123
}).map(|s| {
68124
libc::timespec { tv_sec: s, tv_nsec: nsec }
69-
}).unwrap_or_else(|| {
70-
libc::timespec {
71-
tv_sec: <libc::time_t>::max_value(),
72-
tv_nsec: 1_000_000_000 - 1,
73-
}
74-
});
125+
}).unwrap_or(TIMESPEC_MAX);
75126

76127
// And wait!
77128
let r = libc::pthread_cond_timedwait(self.inner.get(), mutex::raw(mutex),

src/libstd/sys/windows/condvar.rs

+3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ impl Condvar {
2424
Condvar { inner: UnsafeCell::new(c::CONDITION_VARIABLE_INIT) }
2525
}
2626

27+
#[inline]
28+
pub unsafe fn init(&mut self) {}
29+
2730
#[inline]
2831
pub unsafe fn wait(&self, mutex: &Mutex) {
2932
let r = c::SleepConditionVariableSRW(self.inner.get(),

0 commit comments

Comments
 (0)