From 6e92f139b1a070072ef98b1c82f66364e4bca7e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Fri, 14 Dec 2018 22:46:47 +0100 Subject: [PATCH 1/5] Simplify with_thread_data --- core/src/parking_lot.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/core/src/parking_lot.rs b/core/src/parking_lot.rs index ff663a9b..66be2aed 100644 --- a/core/src/parking_lot.rs +++ b/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 }) } From edeaa24336f7fc3bac97a75acfdd36e6814ace63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Sat, 15 Dec 2018 14:30:43 +0100 Subject: [PATCH 2/5] Add test on panicking wait_for --- src/condvar.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/condvar.rs b/src/condvar.rs index eb757d52..d2aa3769 100644 --- a/src/condvar.rs +++ b/src/condvar.rs @@ -555,12 +555,20 @@ 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. + #[cfg(feature = "nightly")] + let very_long_timeout = Duration::from_secs(u64::max_value()); + #[cfg(not(feature = "nightly"))] + let very_long_timeout = 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); } From 6d8fa720f241ee6b399e72c4edcc684e4a335a2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Sat, 15 Dec 2018 14:31:04 +0100 Subject: [PATCH 3/5] Remove risk of panicking in wait_for style functions --- src/condvar.rs | 9 ++++++--- src/lib.rs | 1 + src/raw_mutex.rs | 4 ++-- src/raw_rwlock.rs | 16 ++++++---------- src/util.rs | 12 ++++++++++++ 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/condvar.rs b/src/condvar.rs index d2aa3769..a9923a2e 100644 --- a/src/condvar.rs +++ b/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( &self, - guard: &mut MutexGuard, + mutex_guard: &mut MutexGuard, 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) } } diff --git a/src/lib.rs b/src/lib.rs index f16b8214..e8cd726e 100644 --- a/src/lib.rs +++ b/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; diff --git a/src/raw_mutex.rs b/src/raw_mutex.rs index 36c2b93f..4b5f7625 100644 --- a/src/raw_mutex.rs +++ b/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) }; diff --git a/src/raw_rwlock.rs b/src/raw_rwlock.rs index 2c1d3056..5a4e26b7 100644 --- a/src/raw_rwlock.rs +++ b/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)) } } } diff --git a/src/util.rs b/src/util.rs index c7dfd322..c05a539b 100644 --- a/src/util.rs +++ b/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 { 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 { + #[cfg(feature = "nightly")] + let deadline = Instant::now().checked_add(timeout); + #[cfg(not(feature = "nightly"))] + let deadline = Some(Instant::now() + timeout); + + deadline +} From 69f888569b7bb00edeaee76bd90ed0583513467a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Sat, 15 Dec 2018 15:05:49 +0100 Subject: [PATCH 4/5] Use cfg! where possible --- src/condvar.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/condvar.rs b/src/condvar.rs index a9923a2e..cb85e885 100644 --- a/src/condvar.rs +++ b/src/condvar.rs @@ -564,10 +564,11 @@ mod tests { c2.notify_one(); }); // Non-nightly panics on too large timeouts. Nightly treats it as indefinite wait. - #[cfg(feature = "nightly")] - let very_long_timeout = Duration::from_secs(u64::max_value()); - #[cfg(not(feature = "nightly"))] - let very_long_timeout = Duration::from_millis(u32::max_value() as u64); + 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()); From e593f1023165d70a9e783c698866b67e93e5ebaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Sat, 15 Dec 2018 15:52:37 +0100 Subject: [PATCH 5/5] Skip testing deadlock detection on old Rust versions --- .travis.yml | 3 ++- appveyor.yml | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index dba49d42..8ea2d8c2 100644 --- a/.travis.yml +++ b/.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 diff --git a/appveyor.yml b/appveyor.yml index fef6b570..83498db3 100644 --- a/appveyor.yml +++ b/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