Fix condvar.wait(distant future) return immediately on OSX
Fixes issue #37440: `pthread_cond_timedwait` on macOS Sierra seems to overflow `ts_sec` parameter and returns immediately. To work around this problem patch rounds timeout down to approximately 1000 years. Patch also fixes overflow when converting `u64` to `time_t`.
This commit is contained in:
parent
258ae6dd9b
commit
0c26b5998d
@ -480,9 +480,10 @@ impl Drop for Condvar {
|
||||
mod tests {
|
||||
use sync::mpsc::channel;
|
||||
use sync::{Condvar, Mutex, Arc};
|
||||
use sync::atomic::{AtomicBool, Ordering};
|
||||
use thread;
|
||||
use time::Duration;
|
||||
use u32;
|
||||
use u64;
|
||||
|
||||
#[test]
|
||||
fn smoke() {
|
||||
@ -547,23 +548,58 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
#[cfg_attr(target_os = "emscripten", ignore)]
|
||||
fn wait_timeout_ms() {
|
||||
fn wait_timeout_wait() {
|
||||
let m = Arc::new(Mutex::new(()));
|
||||
let m2 = m.clone();
|
||||
let c = Arc::new(Condvar::new());
|
||||
let c2 = c.clone();
|
||||
|
||||
let g = m.lock().unwrap();
|
||||
let (g, _no_timeout) = c.wait_timeout(g, Duration::from_millis(1)).unwrap();
|
||||
// spurious wakeups mean this isn't necessarily true
|
||||
// assert!(!no_timeout);
|
||||
let _t = thread::spawn(move || {
|
||||
let _g = m2.lock().unwrap();
|
||||
c2.notify_one();
|
||||
});
|
||||
let (g, timeout_res) = c.wait_timeout(g, Duration::from_millis(u32::MAX as u64)).unwrap();
|
||||
assert!(!timeout_res.timed_out());
|
||||
drop(g);
|
||||
loop {
|
||||
let g = m.lock().unwrap();
|
||||
let (_g, no_timeout) = c.wait_timeout(g, Duration::from_millis(1)).unwrap();
|
||||
// spurious wakeups mean this isn't necessarily true
|
||||
// so execute test again, if not timeout
|
||||
if !no_timeout.timed_out() {
|
||||
continue;
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg_attr(target_os = "emscripten", ignore)]
|
||||
fn wait_timeout_wake() {
|
||||
let m = Arc::new(Mutex::new(()));
|
||||
let c = Arc::new(Condvar::new());
|
||||
|
||||
loop {
|
||||
let g = m.lock().unwrap();
|
||||
|
||||
let c2 = c.clone();
|
||||
let m2 = m.clone();
|
||||
|
||||
let notified = Arc::new(AtomicBool::new(false));
|
||||
let notified_copy = notified.clone();
|
||||
|
||||
let t = thread::spawn(move || {
|
||||
let _g = m2.lock().unwrap();
|
||||
thread::sleep(Duration::from_millis(1));
|
||||
notified_copy.store(true, Ordering::SeqCst);
|
||||
c2.notify_one();
|
||||
});
|
||||
let (g, timeout_res) = c.wait_timeout(g, Duration::from_millis(u64::MAX)).unwrap();
|
||||
assert!(!timeout_res.timed_out());
|
||||
// spurious wakeups mean this isn't necessarily true
|
||||
// so execute test again, if not notified
|
||||
if !notified.load(Ordering::SeqCst) {
|
||||
t.join().unwrap();
|
||||
continue;
|
||||
}
|
||||
drop(g);
|
||||
|
||||
t.join().unwrap();
|
||||
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
@ -23,6 +23,14 @@ const TIMESPEC_MAX: libc::timespec = libc::timespec {
|
||||
tv_nsec: 1_000_000_000 - 1,
|
||||
};
|
||||
|
||||
fn saturating_cast_to_time_t(value: u64) -> libc::time_t {
|
||||
if value > <libc::time_t>::max_value() as u64 {
|
||||
<libc::time_t>::max_value()
|
||||
} else {
|
||||
value as libc::time_t
|
||||
}
|
||||
}
|
||||
|
||||
impl Condvar {
|
||||
pub const fn new() -> Condvar {
|
||||
// Might be moved and address is changing it is better to avoid
|
||||
@ -79,8 +87,7 @@ impl Condvar {
|
||||
|
||||
// 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)
|
||||
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;
|
||||
@ -100,10 +107,29 @@ impl Condvar {
|
||||
// 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 {
|
||||
pub unsafe fn wait_timeout(&self, mutex: &Mutex, mut dur: Duration) -> bool {
|
||||
use ptr;
|
||||
use time::Instant;
|
||||
|
||||
// 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.
|
||||
@ -116,7 +142,7 @@ impl Condvar {
|
||||
(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 = dur.as_secs() as libc::time_t;
|
||||
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)
|
||||
|
Loading…
Reference in New Issue
Block a user