Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No overflow panic on very long timeouts #111

Merged
merged 5 commits into from
Dec 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 5 additions & 11 deletions core/src/parking_lot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
}
Expand Down
20 changes: 16 additions & 4 deletions src/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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);
}

Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/raw_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) };
Expand Down
16 changes: 6 additions & 10 deletions src/raw_rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) };
Expand Down Expand Up @@ -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) };
Expand Down Expand Up @@ -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) };
Expand Down Expand Up @@ -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) };
Expand Down Expand Up @@ -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))
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
}