Auto merge of #42604 - stepancheg:timedwait, r=alexcrichton

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 year 3000.

Patch also fixes overflow when converting `u64` to `time_t`.

Author: bors

src/libstd/sync/condvar.rs

@@ -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]

src/libstd/sys/unix/condvar.rs

@@ -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)