Merge pull request #111 from faern/no-instant-overflow-panic

No overflow panic on very long timeouts

.travis.yml

@@ -21,7 +21,8 @@ script:
 - cd ..;
 - travis-cargo build
 - travis-cargo test
-- travis-cargo test -- --features=deadlock_detection
+- travis-cargo --only stable test -- --features=deadlock_detection
+- travis-cargo --only beta test -- --features=deadlock_detection
 - travis-cargo --only nightly doc -- --all-features --no-deps -p parking_lot -p parking_lot_core -p lock_api
 - cd benchmark
 - travis-cargo build

appveyor.yml

@@ -25,5 +25,5 @@ build_script:
 
 test_script:
   - travis-cargo test
-  - travis-cargo test -- --features=deadlock_detection
+  - travis-cargo --only nightly test -- --features=deadlock_detection
   - travis-cargo doc

core/src/parking_lot.rs

@@ -179,18 +179,12 @@ where
     }
 
     // Unlike word_lock::ThreadData, parking_lot::ThreadData is always expensive
-    // to construct. Try to use a thread-local version if possible.
-    let mut thread_data_ptr = ptr::null();
-    thread_local!(static THREAD_DATA: ThreadData = ThreadData::new());
-    if let Some(tls_thread_data) = try_get_tls(&THREAD_DATA) {
-        thread_data_ptr = tls_thread_data;
-    }
-
-    // Otherwise just create a ThreadData on the stack
+    // to construct. Try to use a thread-local version if possible. Otherwise just
+    // create a ThreadData on the stack
     let mut thread_data_storage = None;
-    if thread_data_ptr.is_null() {
-        thread_data_ptr = thread_data_storage.get_or_insert_with(ThreadData::new);
-    }
+    thread_local!(static THREAD_DATA: ThreadData = ThreadData::new());
+    let thread_data_ptr = try_get_tls(&THREAD_DATA)
+        .unwrap_or_else(|| thread_data_storage.get_or_insert_with(ThreadData::new));
 
     f(unsafe { &*thread_data_ptr })
 }

src/condvar.rs

@@ -13,6 +13,7 @@ use raw_mutex::{RawMutex, TOKEN_HANDOFF, TOKEN_NORMAL};
 use std::sync::atomic::{AtomicPtr, Ordering};
 use std::time::{Duration, Instant};
 use std::{fmt, ptr};
+use util;
 
 /// A type indicating whether a timed wait on a condition variable returned
 /// due to a time out or not.
@@ -391,14 +392,16 @@ impl Condvar {
     /// # Panics
     ///
     /// Panics if the given `timeout` is so large that it can't be added to the current time.
+    /// This panic is not possible if the crate is built with the `nightly` feature, then a too
+    /// large `timeout` becomes equivalent to just calling `wait`.
     #[inline]
     pub fn wait_for<T: ?Sized>(
         &self,
-        guard: &mut MutexGuard<T>,
+        mutex_guard: &mut MutexGuard<T>,
         timeout: Duration,
     ) -> WaitTimeoutResult {
-        // FIXME: Change to Instant::now().checked_add(timeout) when stable.
-        self.wait_until(guard, Instant::now() + timeout)
+        let deadline = util::to_deadline(timeout);
+        self.wait_until_internal(unsafe { MutexGuard::mutex(mutex_guard).raw() }, deadline)
     }
 }
 
@@ -555,12 +558,21 @@ mod tests {
         let mut g = m.lock();
         let no_timeout = c.wait_for(&mut g, Duration::from_millis(1));
         assert!(no_timeout.timed_out());
+
         let _t = thread::spawn(move || {
             let _g = m2.lock();
             c2.notify_one();
         });
-        let timeout_res = c.wait_for(&mut g, Duration::from_millis(u32::max_value() as u64));
+        // Non-nightly panics on too large timeouts. Nightly treats it as indefinite wait.
+        let very_long_timeout = if cfg!(feature = "nightly") {
+            Duration::from_secs(u64::max_value())
+        } else {
+            Duration::from_millis(u32::max_value() as u64)
+        };
+
+        let timeout_res = c.wait_for(&mut g, very_long_timeout);
         assert!(!timeout_res.timed_out());
+
         drop(g);
     }
 

src/lib.rs

@@ -13,6 +13,7 @@
 #![cfg_attr(feature = "nightly", feature(const_fn))]
 #![cfg_attr(feature = "nightly", feature(integer_atomics))]
 #![cfg_attr(feature = "nightly", feature(asm))]
+#![cfg_attr(feature = "nightly", feature(time_checked_add))]
 
 extern crate lock_api;
 extern crate parking_lot_core;

src/raw_mutex.rs

@@ -20,6 +20,7 @@ use deadlock;
 use lock_api::{GuardNoSend, RawMutex as RawMutexTrait, RawMutexFair, RawMutexTimed};
 use parking_lot_core::{self, ParkResult, SpinWait, UnparkResult, UnparkToken, DEFAULT_PARK_TOKEN};
 use std::time::{Duration, Instant};
+use util;
 
 // UnparkToken used to indicate that that the target thread should attempt to
 // lock the mutex again as soon as it is unparked.
@@ -144,8 +145,7 @@ unsafe impl RawMutexTimed for RawMutex {
         {
             true
         } else {
-            // FIXME: Change to Intstant::now().checked_add(timeout) when stable.
-            self.lock_slow(Some(Instant::now() + timeout))
+            self.lock_slow(util::to_deadline(timeout))
         };
         if result {
             unsafe { deadlock::acquire_resource(self as *const _ as usize) };

src/raw_rwlock.rs

@@ -17,6 +17,7 @@ use raw_mutex::{TOKEN_HANDOFF, TOKEN_NORMAL};
 use std::cell::Cell;
 use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT};
 use std::time::{Duration, Instant};
+use util;
 
 const PARKED_BIT: usize = 0b001;
 const UPGRADING_BIT: usize = 0b010;
@@ -230,8 +231,7 @@ unsafe impl RawRwLockTimed for RawRwLock {
         let result = if self.try_lock_shared_fast(false) {
             true
         } else {
-            // FIXME: Change to Instant::now().checked_add(timeout) when stable.
-            self.lock_shared_slow(false, Some(Instant::now() + timeout))
+            self.lock_shared_slow(false, util::to_deadline(timeout))
         };
         if result {
             unsafe { deadlock::acquire_resource(self as *const _ as usize) };
@@ -261,8 +261,7 @@ unsafe impl RawRwLockTimed for RawRwLock {
         {
             true
         } else {
-            // FIXME: Change to Instant::now().checked_add(timeout) when stable.
-            self.lock_exclusive_slow(Some(Instant::now() + timeout))
+            self.lock_exclusive_slow(util::to_deadline(timeout))
         };
         if result {
             unsafe { deadlock::acquire_resource(self as *const _ as usize) };
@@ -318,8 +317,7 @@ unsafe impl RawRwLockRecursiveTimed for RawRwLock {
         let result = if self.try_lock_shared_fast(true) {
             true
         } else {
-            // FIXME: Change to Instant::now().checked_add(timeout) when stable.
-            self.lock_shared_slow(true, Some(Instant::now() + timeout))
+            self.lock_shared_slow(true, util::to_deadline(timeout))
         };
         if result {
             unsafe { deadlock::acquire_resource(self as *const _ as usize) };
@@ -479,8 +477,7 @@ unsafe impl RawRwLockUpgradeTimed for RawRwLock {
         let result = if self.try_lock_upgradable_fast() {
             true
         } else {
-            // FIXME: Change to Instant::now().checked_add(timeout) when stable.
-            self.lock_upgradable_slow(Some(Instant::now() + timeout))
+            self.lock_upgradable_slow(util::to_deadline(timeout))
         };
         if result {
             unsafe { deadlock::acquire_resource(self as *const _ as usize) };
@@ -520,8 +517,7 @@ unsafe impl RawRwLockUpgradeTimed for RawRwLock {
         {
             true
         } else {
-            // FIXME: Change to Instant::now().checked_add(timeout) when stable.
-            self.upgrade_slow(Some(Instant::now() + timeout))
+            self.upgrade_slow(util::to_deadline(timeout))
         }
     }
 }

src/util.rs

@@ -5,6 +5,8 @@
 // http://opensource.org/licenses/MIT>, at your option. This file may not be
 // copied, modified, or distributed except according to those terms.
 
+use std::time::{Duration, Instant};
+
 // Option::unchecked_unwrap
 pub trait UncheckedOptionExt<T> {
     unsafe fn unchecked_unwrap(self) -> T;
@@ -30,3 +32,13 @@ unsafe fn unreachable() -> ! {
         match *(1 as *const Void) {}
     }
 }
+
+#[inline]
+pub fn to_deadline(timeout: Duration) -> Option<Instant> {
+    #[cfg(feature = "nightly")]
+    let deadline = Instant::now().checked_add(timeout);
+    #[cfg(not(feature = "nightly"))]
+    let deadline = Some(Instant::now() + timeout);
+
+    deadline
+}