Skip to content

Make Condvar::wait_timeout return an enum instead of a bool #27826

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

Merged
merged 4 commits into from
Aug 22, 2015
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
61 changes: 41 additions & 20 deletions src/libstd/sync/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@ use sys_common::poison::{self, LockResult};
use sys::time::SteadyTime;
use time::Duration;

/// A type indicating whether a timed wait on a condition variable returned
/// due to a time out or not.
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
#[unstable(feature = "wait_timeout", reason = "newly added", issue = "27772")]
pub struct WaitTimeoutResult(bool);

impl WaitTimeoutResult {
/// Returns whether the wait was known to have timed out.
#[unstable(feature = "wait_timeout", reason = "newly added", issue = "27772")]
pub fn timed_out(&self) -> bool {
self.0
}
}

/// A Condition Variable
///
/// Condition variables represent the ability to block a thread such that it
Expand Down Expand Up @@ -170,16 +184,16 @@ impl Condvar {
/// preemption or platform differences that may not cause the maximum
/// amount of time waited to be precisely `dur`.
///
/// The returned boolean is `false` only if the timeout is known
/// to have elapsed.
/// The returned `WaitTimeoutResult` value indicates if the timeout is
/// known to have elapsed.
///
/// Like `wait`, the lock specified will be re-acquired when this function
/// returns, regardless of whether the timeout elapsed or not.
#[unstable(feature = "wait_timeout", reason = "waiting for Duration",
issue = "27772")]
pub fn wait_timeout<'a, T>(&self, guard: MutexGuard<'a, T>,
dur: Duration)
-> LockResult<(MutexGuard<'a, T>, bool)> {
-> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)> {
unsafe {
let me: &'static Condvar = &*(self as *const _);
me.inner.wait_timeout(guard, dur)
Expand All @@ -199,7 +213,7 @@ impl Condvar {
guard: MutexGuard<'a, T>,
dur: Duration,
f: F)
-> LockResult<(MutexGuard<'a, T>, bool)>
-> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)>
where F: FnMut(LockResult<&mut T>) -> bool {
unsafe {
let me: &'static Condvar = &*(self as *const _);
Expand Down Expand Up @@ -278,7 +292,13 @@ impl StaticCondvar {
issue = "27717")]
pub fn wait_timeout_ms<'a, T>(&'static self, guard: MutexGuard<'a, T>, ms: u32)
-> LockResult<(MutexGuard<'a, T>, bool)> {
self.wait_timeout(guard, Duration::from_millis(ms as u64))
match self.wait_timeout(guard, Duration::from_millis(ms as u64)) {
Ok((guard, timed_out)) => Ok((guard, !timed_out.timed_out())),
Err(poison) => {
let (guard, timed_out) = poison.into_inner();
Err(PoisonError::new((guard, !timed_out.timed_out())))
}
}
}

/// Waits on this condition variable for a notification, timing out after a
Expand All @@ -291,17 +311,17 @@ impl StaticCondvar {
pub fn wait_timeout<'a, T>(&'static self,
guard: MutexGuard<'a, T>,
timeout: Duration)
-> LockResult<(MutexGuard<'a, T>, bool)> {
let (poisoned, success) = unsafe {
-> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)> {
let (poisoned, result) = unsafe {
let lock = mutex::guard_lock(&guard);
self.verify(lock);
let success = self.inner.wait_timeout(lock, timeout);
(mutex::guard_poison(&guard).get(), success)
(mutex::guard_poison(&guard).get(), WaitTimeoutResult(!success))
};
if poisoned {
Err(PoisonError::new((guard, success)))
Err(PoisonError::new((guard, result)))
} else {
Ok((guard, success))
Ok((guard, result))
}
}

Expand All @@ -319,7 +339,7 @@ impl StaticCondvar {
guard: MutexGuard<'a, T>,
dur: Duration,
mut f: F)
-> LockResult<(MutexGuard<'a, T>, bool)>
-> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)>
where F: FnMut(LockResult<&mut T>) -> bool {
// This could be made more efficient by pushing the implementation into
// sys::condvar
Expand All @@ -332,28 +352,29 @@ impl StaticCondvar {
let now = SteadyTime::now();
let consumed = &now - &start;
let guard = guard_result.unwrap_or_else(|e| e.into_inner());
let (new_guard_result, no_timeout) = if consumed > dur {
(Ok(guard), false)
let (new_guard_result, timed_out) = if consumed > dur {
(Ok(guard), WaitTimeoutResult(true))
} else {
match self.wait_timeout(guard, dur - consumed) {
Ok((new_guard, no_timeout)) => (Ok(new_guard), no_timeout),
Ok((new_guard, timed_out)) => (Ok(new_guard), timed_out),
Err(err) => {
let (new_guard, no_timeout) = err.into_inner();
(Err(PoisonError::new(new_guard)), no_timeout)
}
}
};
guard_result = new_guard_result;
if !no_timeout {
if timed_out.timed_out() {
let result = f(guard_result
.as_mut()
.map(|g| &mut **g)
.map_err(|e| PoisonError::new(&mut **e.get_mut())));
let result = WaitTimeoutResult(!result);
return poison::map_result(guard_result, |g| (g, result));
}
}

poison::map_result(guard_result, |g| (g, true))
poison::map_result(guard_result, |g| (g, WaitTimeoutResult(false)))
}

/// Wakes up one blocked thread on this condvar.
Expand Down Expand Up @@ -508,10 +529,10 @@ mod tests {
static S: AtomicUsize = AtomicUsize::new(0);

let g = M.lock().unwrap();
let (g, success) = C.wait_timeout_with(g, Duration::new(0, 1000), |_| {
let (g, timed_out) = C.wait_timeout_with(g, Duration::new(0, 1000), |_| {
false
}).unwrap();
assert!(!success);
assert!(timed_out.timed_out());

let (tx, rx) = channel();
let _t = thread::spawn(move || {
Expand All @@ -535,7 +556,7 @@ mod tests {

let mut state = 0;
let day = 24 * 60 * 60;
let (_g, success) = C.wait_timeout_with(g, Duration::new(day, 0), |_| {
let (_g, timed_out) = C.wait_timeout_with(g, Duration::new(day, 0), |_| {
assert_eq!(state, S.load(Ordering::SeqCst));
tx.send(()).unwrap();
state += 1;
Expand All @@ -544,7 +565,7 @@ mod tests {
_ => true,
}
}).unwrap();
assert!(success);
assert!(!timed_out.timed_out());
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub use alloc::arc::{Arc, Weak};
pub use core::atomic;

pub use self::barrier::{Barrier, BarrierWaitResult};
pub use self::condvar::{Condvar, StaticCondvar, CONDVAR_INIT};
pub use self::condvar::{Condvar, StaticCondvar, WaitTimeoutResult, CONDVAR_INIT};
pub use self::mutex::MUTEX_INIT;
pub use self::mutex::{Mutex, MutexGuard, StaticMutex};
pub use self::once::{Once, ONCE_INIT};
Expand Down