diff --git a/src/doc/intro.md b/src/doc/intro.md index 5be97034357..a4e9d85bffd 100644 --- a/src/doc/intro.md +++ b/src/doc/intro.md @@ -483,7 +483,7 @@ fn main() { for i in range(0u, 3u) { let number = numbers.clone(); Thread::spawn(move || { - let mut array = number.lock(); + let mut array = number.lock().unwrap(); (*array)[i] += 1; diff --git a/src/liballoc/arc.rs b/src/liballoc/arc.rs index 3e235caab18..13ee8c26075 100644 --- a/src/liballoc/arc.rs +++ b/src/liballoc/arc.rs @@ -58,7 +58,7 @@ //! let five = five.clone(); //! //! Thread::spawn(move || { -//! let mut number = five.lock(); +//! let mut number = five.lock().unwrap(); //! //! *number += 1; //! @@ -722,7 +722,7 @@ mod tests { let a = Arc::new(Cycle { x: Mutex::new(None) }); let b = a.clone().downgrade(); - *a.x.lock() = Some(b); + *a.x.lock().unwrap() = Some(b); // hopefully we don't double-free (or leak)... } diff --git a/src/librustc_trans/back/write.rs b/src/librustc_trans/back/write.rs index 513b955da3f..99e11bf5202 100644 --- a/src/librustc_trans/back/write.rs +++ b/src/librustc_trans/back/write.rs @@ -98,7 +98,7 @@ impl SharedEmitter { } fn dump(&mut self, handler: &Handler) { - let mut buffer = self.buffer.lock(); + let mut buffer = self.buffer.lock().unwrap(); for diag in buffer.iter() { match diag.code { Some(ref code) => { @@ -123,7 +123,7 @@ impl Emitter for SharedEmitter { msg: &str, code: Option<&str>, lvl: Level) { assert!(cmsp.is_none(), "SharedEmitter doesn't support spans"); - self.buffer.lock().push(Diagnostic { + self.buffer.lock().unwrap().push(Diagnostic { msg: msg.to_string(), code: code.map(|s| s.to_string()), lvl: lvl, @@ -915,7 +915,7 @@ fn run_work_multithreaded(sess: &Session, loop { // Avoid holding the lock for the entire duration of the match. - let maybe_work = work_items_arc.lock().pop(); + let maybe_work = work_items_arc.lock().unwrap().pop(); match maybe_work { Some(work) => { execute_work_item(&cgcx, work); diff --git a/src/libstd/comm/shared.rs b/src/libstd/comm/shared.rs index 1022694e634..3f23ec5dc66 100644 --- a/src/libstd/comm/shared.rs +++ b/src/libstd/comm/shared.rs @@ -86,7 +86,7 @@ impl Packet { // and that could cause problems on platforms where it is // represented by opaque data structure pub fn postinit_lock(&self) -> MutexGuard<()> { - self.select_lock.lock() + self.select_lock.lock().unwrap() } // This function is used at the creation of a shared packet to inherit a @@ -435,7 +435,7 @@ impl Packet { // about looking at and dealing with to_wake. Once we have acquired the // lock, we are guaranteed that inherit_blocker is done. { - let _guard = self.select_lock.lock(); + let _guard = self.select_lock.lock().unwrap(); } // Like the stream implementation, we want to make sure that the count diff --git a/src/libstd/comm/sync.rs b/src/libstd/comm/sync.rs index 88338849965..82ec1814ebd 100644 --- a/src/libstd/comm/sync.rs +++ b/src/libstd/comm/sync.rs @@ -121,9 +121,9 @@ fn wait<'a, 'b, T: Send>(lock: &'a Mutex>, NoneBlocked => {} _ => unreachable!(), } - drop(guard); // unlock - wait_token.wait(); // block - lock.lock() // relock + drop(guard); // unlock + wait_token.wait(); // block + lock.lock().unwrap() // relock } /// Wakes up a thread, dropping the lock at the correct time @@ -161,7 +161,7 @@ impl Packet { fn acquire_send_slot(&self) -> MutexGuard> { let mut node = Node { token: None, next: 0 as *mut Node }; loop { - let mut guard = self.lock.lock(); + let mut guard = self.lock.lock().unwrap(); // are we ready to go? if guard.disconnected || guard.buf.size() < guard.buf.cap() { return guard; @@ -202,7 +202,7 @@ impl Packet { } pub fn try_send(&self, t: T) -> Result<(), super::TrySendError> { - let mut guard = self.lock.lock(); + let mut guard = self.lock.lock().unwrap(); if guard.disconnected { Err(super::RecvDisconnected(t)) } else if guard.buf.size() == guard.buf.cap() { @@ -239,7 +239,7 @@ impl Packet { // When reading this, remember that there can only ever be one receiver at // time. pub fn recv(&self) -> Result { - let mut guard = self.lock.lock(); + let mut guard = self.lock.lock().unwrap(); // Wait for the buffer to have something in it. No need for a while loop // because we're the only receiver. @@ -258,7 +258,7 @@ impl Packet { } pub fn try_recv(&self) -> Result { - let mut guard = self.lock.lock(); + let mut guard = self.lock.lock().unwrap(); // Easy cases first if guard.disconnected { return Err(Disconnected) } @@ -315,7 +315,7 @@ impl Packet { } // Not much to do other than wake up a receiver if one's there - let mut guard = self.lock.lock(); + let mut guard = self.lock.lock().unwrap(); if guard.disconnected { return } guard.disconnected = true; match mem::replace(&mut guard.blocker, NoneBlocked) { @@ -326,7 +326,7 @@ impl Packet { } pub fn drop_port(&self) { - let mut guard = self.lock.lock(); + let mut guard = self.lock.lock().unwrap(); if guard.disconnected { return } guard.disconnected = true; @@ -372,14 +372,14 @@ impl Packet { // If Ok, the value is whether this port has data, if Err, then the upgraded // port needs to be checked instead of this one. pub fn can_recv(&self) -> bool { - let guard = self.lock.lock(); + let guard = self.lock.lock().unwrap(); guard.disconnected || guard.buf.size() > 0 } // Attempts to start selection on this port. This can either succeed or fail // because there is data waiting. pub fn start_selection(&self, token: SignalToken) -> StartResult { - let mut guard = self.lock.lock(); + let mut guard = self.lock.lock().unwrap(); if guard.disconnected || guard.buf.size() > 0 { Abort } else { @@ -397,7 +397,7 @@ impl Packet { // // The return value indicates whether there's data on this port. pub fn abort_selection(&self) -> bool { - let mut guard = self.lock.lock(); + let mut guard = self.lock.lock().unwrap(); match mem::replace(&mut guard.blocker, NoneBlocked) { NoneBlocked => true, BlockedSender(token) => { @@ -413,7 +413,7 @@ impl Packet { impl Drop for Packet { fn drop(&mut self) { assert_eq!(self.channels.load(atomic::SeqCst), 0); - let mut guard = self.lock.lock(); + let mut guard = self.lock.lock().unwrap(); assert!(guard.queue.dequeue().is_none()); assert!(guard.canceled.is_none()); } diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index 6bd721599f3..6c8e4eea40f 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -146,7 +146,7 @@ impl StdinReader { /// ``` pub fn lock<'a>(&'a mut self) -> StdinReaderGuard<'a> { StdinReaderGuard { - inner: self.inner.lock() + inner: self.inner.lock().unwrap() } } @@ -155,7 +155,7 @@ impl StdinReader { /// The read is performed atomically - concurrent read calls in other /// threads will not interleave with this one. pub fn read_line(&mut self) -> IoResult { - self.inner.lock().0.read_line() + self.inner.lock().unwrap().0.read_line() } /// Like `Buffer::read_until`. @@ -163,7 +163,7 @@ impl StdinReader { /// The read is performed atomically - concurrent read calls in other /// threads will not interleave with this one. pub fn read_until(&mut self, byte: u8) -> IoResult> { - self.inner.lock().0.read_until(byte) + self.inner.lock().unwrap().0.read_until(byte) } /// Like `Buffer::read_char`. @@ -171,13 +171,13 @@ impl StdinReader { /// The read is performed atomically - concurrent read calls in other /// threads will not interleave with this one. pub fn read_char(&mut self) -> IoResult { - self.inner.lock().0.read_char() + self.inner.lock().unwrap().0.read_char() } } impl Reader for StdinReader { fn read(&mut self, buf: &mut [u8]) -> IoResult { - self.inner.lock().0.read(buf) + self.inner.lock().unwrap().0.read(buf) } // We have to manually delegate all of these because the default impls call @@ -185,23 +185,23 @@ impl Reader for StdinReader { // incur the costs of repeated locking). fn read_at_least(&mut self, min: uint, buf: &mut [u8]) -> IoResult { - self.inner.lock().0.read_at_least(min, buf) + self.inner.lock().unwrap().0.read_at_least(min, buf) } fn push_at_least(&mut self, min: uint, len: uint, buf: &mut Vec) -> IoResult { - self.inner.lock().0.push_at_least(min, len, buf) + self.inner.lock().unwrap().0.push_at_least(min, len, buf) } fn read_to_end(&mut self) -> IoResult> { - self.inner.lock().0.read_to_end() + self.inner.lock().unwrap().0.read_to_end() } fn read_le_uint_n(&mut self, nbytes: uint) -> IoResult { - self.inner.lock().0.read_le_uint_n(nbytes) + self.inner.lock().unwrap().0.read_le_uint_n(nbytes) } fn read_be_uint_n(&mut self, nbytes: uint) -> IoResult { - self.inner.lock().0.read_be_uint_n(nbytes) + self.inner.lock().unwrap().0.read_be_uint_n(nbytes) } } diff --git a/src/libstd/sync/barrier.rs b/src/libstd/sync/barrier.rs index 6cdb199819a..4091f0df395 100644 --- a/src/libstd/sync/barrier.rs +++ b/src/libstd/sync/barrier.rs @@ -69,7 +69,7 @@ impl Barrier { /// Barriers are re-usable after all threads have rendezvoused once, and can /// be used continuously. pub fn wait(&self) { - let mut lock = self.lock.lock(); + let mut lock = self.lock.lock().unwrap(); let local_gen = lock.generation_id; lock.count += 1; if lock.count < self.num_threads { @@ -77,7 +77,7 @@ impl Barrier { // http://en.wikipedia.org/wiki/Spurious_wakeup while local_gen == lock.generation_id && lock.count < self.num_threads { - self.cvar.wait(&lock); + lock = self.cvar.wait(lock).unwrap(); } } else { lock.count = 0; diff --git a/src/libstd/sync/condvar.rs b/src/libstd/sync/condvar.rs index f1940bfd829..3e17d8b6be1 100644 --- a/src/libstd/sync/condvar.rs +++ b/src/libstd/sync/condvar.rs @@ -11,7 +11,8 @@ use prelude::*; use sync::atomic::{mod, AtomicUint}; -use sync::{mutex, StaticMutexGuard}; +use sync::poison::{mod, LockResult}; +use sync::CondvarGuard; use sys_common::condvar as sys; use sys_common::mutex as sys_mutex; use time::Duration; @@ -44,16 +45,16 @@ use time::Duration; /// // Inside of our lock, spawn a new thread, and then wait for it to start /// Thread::spawn(move|| { /// let &(ref lock, ref cvar) = &*pair2; -/// let mut started = lock.lock(); +/// let mut started = lock.lock().unwrap(); /// *started = true; /// cvar.notify_one(); /// }).detach(); /// /// // wait for the thread to start up /// let &(ref lock, ref cvar) = &*pair; -/// let started = lock.lock(); +/// let mut started = lock.lock().unwrap(); /// while !*started { -/// cvar.wait(&started); +/// started = cvar.wait(started).unwrap(); /// } /// ``` pub struct Condvar { inner: Box } @@ -92,9 +93,9 @@ pub const CONDVAR_INIT: StaticCondvar = StaticCondvar { /// /// Note that this trait should likely not be implemented manually unless you /// really know what you're doing. -pub trait AsMutexGuard { +pub trait AsGuard { #[allow(missing_docs)] - unsafe fn as_mutex_guard(&self) -> &StaticMutexGuard; + fn as_guard(&self) -> CondvarGuard; } impl Condvar { @@ -113,8 +114,8 @@ impl Condvar { /// notification. /// /// This function will atomically unlock the mutex specified (represented by - /// `guard`) and block the current thread. This means that any calls to - /// `notify_*()` which happen logically after the mutex is unlocked are + /// `mutex_guard`) and block the current thread. This means that any calls + /// to `notify_*()` which happen logically after the mutex is unlocked are /// candidates to wake this thread up. When this function call returns, the /// lock specified will have been re-acquired. /// @@ -123,13 +124,20 @@ impl Condvar { /// the predicate must always be checked each time this function returns to /// protect against spurious wakeups. /// + /// # Failure + /// + /// This function will return an error if the mutex being waited on is + /// poisoned when this thread re-acquires the lock. For more information, + /// see information about poisoning on the Mutex type. + /// /// # Panics /// /// This function will `panic!()` if it is used with more than one mutex /// over time. Each condition variable is dynamically bound to exactly one /// mutex to ensure defined behavior across platforms. If this functionality /// is not desired, then unsafe primitives in `sys` are provided. - pub fn wait(&self, mutex_guard: &T) { + pub fn wait(&self, mutex_guard: T) + -> LockResult { unsafe { let me: &'static Condvar = &*(self as *const _); me.inner.wait(mutex_guard) @@ -156,8 +164,8 @@ impl Condvar { // provide. There are also additional concerns about the unix-specific // implementation which may need to be addressed. #[allow(dead_code)] - fn wait_timeout(&self, mutex_guard: &T, - dur: Duration) -> bool { + fn wait_timeout(&self, mutex_guard: T, dur: Duration) + -> LockResult<(T, bool)> { unsafe { let me: &'static Condvar = &*(self as *const _); me.inner.wait_timeout(mutex_guard, dur) @@ -194,13 +202,17 @@ impl StaticCondvar { /// notification. /// /// See `Condvar::wait`. - pub fn wait(&'static self, mutex_guard: &T) { - unsafe { - let lock = mutex_guard.as_mutex_guard(); - let sys = mutex::guard_lock(lock); - self.verify(sys); - self.inner.wait(sys); - (*mutex::guard_poison(lock)).check("mutex"); + pub fn wait(&'static self, mutex_guard: T) -> LockResult { + let poisoned = unsafe { + let cvar_guard = mutex_guard.as_guard(); + self.verify(cvar_guard.lock); + self.inner.wait(cvar_guard.lock); + cvar_guard.poisoned.get() + }; + if poisoned { + Err(poison::new_poison_error(mutex_guard)) + } else { + Ok(mutex_guard) } } @@ -209,15 +221,18 @@ impl StaticCondvar { /// /// See `Condvar::wait_timeout`. #[allow(dead_code)] // may want to stabilize this later, see wait_timeout above - fn wait_timeout(&'static self, mutex_guard: &T, - dur: Duration) -> bool { - unsafe { - let lock = mutex_guard.as_mutex_guard(); - let sys = mutex::guard_lock(lock); - self.verify(sys); - let ret = self.inner.wait_timeout(sys, dur); - (*mutex::guard_poison(lock)).check("mutex"); - return ret; + fn wait_timeout(&'static self, mutex_guard: T, dur: Duration) + -> LockResult<(T, bool)> { + let (poisoned, success) = unsafe { + let cvar_guard = mutex_guard.as_guard(); + self.verify(cvar_guard.lock); + let success = self.inner.wait_timeout(cvar_guard.lock, dur); + (cvar_guard.poisoned.get(), success) + }; + if poisoned { + Err(poison::new_poison_error((mutex_guard, success))) + } else { + Ok((mutex_guard, success)) } } @@ -288,12 +303,12 @@ mod tests { static C: StaticCondvar = CONDVAR_INIT; static M: StaticMutex = MUTEX_INIT; - let g = M.lock(); + let g = M.lock().unwrap(); spawn(move|| { - let _g = M.lock(); + let _g = M.lock().unwrap(); C.notify_one(); }); - C.wait(&g); + let g = C.wait(g).unwrap(); drop(g); unsafe { C.destroy(); M.destroy(); } } @@ -309,13 +324,13 @@ mod tests { let tx = tx.clone(); spawn(move|| { let &(ref lock, ref cond) = &*data; - let mut cnt = lock.lock(); + let mut cnt = lock.lock().unwrap(); *cnt += 1; if *cnt == N { tx.send(()); } while *cnt != 0 { - cond.wait(&cnt); + cnt = cond.wait(cnt).unwrap(); } tx.send(()); }); @@ -324,7 +339,7 @@ mod tests { let &(ref lock, ref cond) = &*data; rx.recv(); - let mut cnt = lock.lock(); + let mut cnt = lock.lock().unwrap(); *cnt = 0; cond.notify_all(); drop(cnt); @@ -339,13 +354,15 @@ mod tests { static C: StaticCondvar = CONDVAR_INIT; static M: StaticMutex = MUTEX_INIT; - let g = M.lock(); - assert!(!C.wait_timeout(&g, Duration::nanoseconds(1000))); + let g = M.lock().unwrap(); + let (g, success) = C.wait_timeout(g, Duration::nanoseconds(1000)).unwrap(); + assert!(!success); spawn(move|| { - let _g = M.lock(); + let _g = M.lock().unwrap(); C.notify_one(); }); - assert!(C.wait_timeout(&g, Duration::days(1))); + let (g, success) = C.wait_timeout(g, Duration::days(1)).unwrap(); + assert!(success); drop(g); unsafe { C.destroy(); M.destroy(); } } @@ -357,15 +374,15 @@ mod tests { static M2: StaticMutex = MUTEX_INIT; static C: StaticCondvar = CONDVAR_INIT; - let g = M1.lock(); + let mut g = M1.lock().unwrap(); spawn(move|| { - let _g = M1.lock(); + let _g = M1.lock().unwrap(); C.notify_one(); }); - C.wait(&g); + g = C.wait(g).unwrap(); drop(g); - C.wait(&M2.lock()); + C.wait(M2.lock().unwrap()).unwrap(); } } diff --git a/src/libstd/sync/mod.rs b/src/libstd/sync/mod.rs index 7605a6a96a0..3f95eac5090 100644 --- a/src/libstd/sync/mod.rs +++ b/src/libstd/sync/mod.rs @@ -17,16 +17,20 @@ #![experimental] +use sys_common::mutex as sys_mutex; + pub use alloc::arc::{Arc, Weak}; -pub use self::mutex::{Mutex, MutexGuard, StaticMutex, StaticMutexGuard, MUTEX_INIT}; +pub use self::mutex::{Mutex, MutexGuard, StaticMutex, StaticMutexGuard}; +pub use self::mutex::MUTEX_INIT; pub use self::rwlock::{RWLock, StaticRWLock, RWLOCK_INIT}; pub use self::rwlock::{RWLockReadGuard, RWLockWriteGuard}; pub use self::rwlock::{StaticRWLockReadGuard, StaticRWLockWriteGuard}; -pub use self::condvar::{Condvar, StaticCondvar, CONDVAR_INIT, AsMutexGuard}; +pub use self::condvar::{Condvar, StaticCondvar, CONDVAR_INIT, AsGuard}; pub use self::once::{Once, ONCE_INIT}; pub use self::semaphore::{Semaphore, SemaphoreGuard}; pub use self::barrier::Barrier; +pub use self::poison::{PoisonError, TryLockError, TryLockResult, LockResult}; pub use self::future::Future; pub use self::task_pool::TaskPool; @@ -41,3 +45,10 @@ mod poison; mod rwlock; mod semaphore; mod task_pool; + +/// Structure returned by `AsGuard` to wait on a condition variable. +// NB: defined here to all modules have access to these private fields. +pub struct CondvarGuard<'a> { + lock: &'a sys_mutex::Mutex, + poisoned: &'a poison::Flag, +} diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index 4d2fbfc4055..621d7274062 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -12,7 +12,8 @@ use prelude::*; use cell::UnsafeCell; use kinds::marker; -use sync::{poison, AsMutexGuard}; +use sync::{AsGuard, CondvarGuard}; +use sync::poison::{mod, TryLockError, TryLockResult, LockResult}; use sys_common::mutex as sys; /// A mutual exclusion primitive useful for protecting shared data @@ -26,12 +27,23 @@ use sys_common::mutex as sys; /// /// # Poisoning /// -/// In order to prevent access to otherwise invalid data, each mutex will -/// propagate any panics which occur while the lock is held. Once a thread has -/// panicked while holding the lock, then all other threads will immediately -/// panic as well once they hold the lock. +/// The mutexes in this module implement a strategy called "poisoning" where a +/// mutex is considered poisoned whenever a thread panics while holding the +/// lock. Once a mutex is poisoned, all other tasks are unable to access the +/// data by default as it is likely tainted (some invariant is not being +/// upheld). /// -/// # Example +/// For a mutex, this means that the `lock` and `try_lock` methods return a +/// `Result` which indicates whether a mutex has been poisoned or not. Most +/// usage of a mutex will simply `unwrap()` these results, propagating panics +/// among threads to ensure that a possibly invalid invariant is not witnessed. +/// +/// A poisoned mutex, however, does not prevent all access to the underlying +/// data. The `PoisonError` type has an `into_guard` method which will return +/// the guard that would have otherwise been returned on a successful lock. This +/// allows access to the data, despite the lock being poisoned. +/// +/// # Examples /// /// ```rust /// use std::sync::{Arc, Mutex}; @@ -48,11 +60,14 @@ use sys_common::mutex as sys; /// let (tx, rx) = channel(); /// for _ in range(0u, 10) { /// let (data, tx) = (data.clone(), tx.clone()); -/// Thread::spawn(move|| { +/// Thread::spawn(move || { /// // The shared static can only be accessed once the lock is held. /// // Our non-atomic increment is safe because we're the only thread /// // which can access the shared state when the lock is held. -/// let mut data = data.lock(); +/// // +/// // We unwrap() the return value to assert that we are not expecting +/// // tasks to ever fail while holding the lock. +/// let mut data = data.lock().unwrap(); /// *data += 1; /// if *data == N { /// tx.send(()); @@ -63,6 +78,35 @@ use sys_common::mutex as sys; /// /// rx.recv(); /// ``` +/// +/// To recover from a poisoned mutex: +/// +/// ```rust +/// use std::sync::{Arc, Mutex}; +/// use std::thread::Thread; +/// +/// let lock = Arc::new(Mutex::new(0u)); +/// let lock2 = lock.clone(); +/// +/// let _ = Thread::spawn(move || -> () { +/// // This thread will acquire the mutex first, unwrapping the result of +/// // `lock` because the lock has not been poisoned. +/// let _lock = lock2.lock().unwrap(); +/// +/// // This panic while holding the lock (`_guard` is in scope) will poison +/// // the mutex. +/// panic!(); +/// }).join(); +/// +/// // The lock is poisoned by this point, but the returned result can be +/// // pattern matched on to return the underlying guard on both branches. +/// let mut guard = match lock.lock() { +/// Ok(guard) => guard, +/// Err(poisoned) => poisoned.into_guard(), +/// }; +/// +/// *guard += 1; +/// ``` pub struct Mutex { // Note that this static mutex is in a *box*, not inlined into the struct // itself. Once a native mutex has been used once, its address can never @@ -93,14 +137,14 @@ unsafe impl Sync for Mutex { } /// static LOCK: StaticMutex = MUTEX_INIT; /// /// { -/// let _g = LOCK.lock(); +/// let _g = LOCK.lock().unwrap(); /// // do some productive work /// } /// // lock is unlocked here. /// ``` pub struct StaticMutex { lock: sys::Mutex, - poison: UnsafeCell, + poison: poison::Flag, } unsafe impl Sync for StaticMutex {} @@ -114,24 +158,27 @@ unsafe impl Sync for StaticMutex {} pub struct MutexGuard<'a, T: 'a> { // funny underscores due to how Deref/DerefMut currently work (they // disregard field privacy). - __lock: &'a Mutex, - __guard: StaticMutexGuard, + __inner: Guard<'a, Mutex>, } /// An RAII implementation of a "scoped lock" of a static mutex. When this /// structure is dropped (falls out of scope), the lock will be unlocked. #[must_use] pub struct StaticMutexGuard { - lock: &'static sys::Mutex, - marker: marker::NoSend, - poison: poison::Guard<'static>, + inner: Guard<'static, StaticMutex>, +} + +struct Guard<'a, T: 'a> { + inner: &'a T, + poison: poison::Guard, + marker: marker::NoSend, // even if 'a is static, this cannot be sent } /// Static initialization of a mutex. This constant can be used to initialize /// other mutex constants. pub const MUTEX_INIT: StaticMutex = StaticMutex { lock: sys::MUTEX_INIT, - poison: UnsafeCell { value: poison::Flag { failed: false } }, + poison: poison::FLAG_INIT, }; impl Mutex { @@ -150,15 +197,13 @@ impl Mutex { /// held. An RAII guard is returned to allow scoped unlock of the lock. When /// the guard goes out of scope, the mutex will be unlocked. /// - /// # Panics + /// # Failure /// /// If another user of this mutex panicked while holding the mutex, then - /// this call will immediately panic once the mutex is acquired. - pub fn lock(&self) -> MutexGuard { - unsafe { - let lock: &'static StaticMutex = &*(&*self.inner as *const _); - MutexGuard::new(self, lock.lock()) - } + /// this call will return an error once the mutex is acquired. + pub fn lock(&self) -> LockResult> { + unsafe { self.inner.lock.lock() } + MutexGuard::new(self) } /// Attempts to acquire this lock. @@ -169,17 +214,16 @@ impl Mutex { /// /// This function does not block. /// - /// # Panics + /// # Failure /// /// If another user of this mutex panicked while holding the mutex, then - /// this call will immediately panic if the mutex would otherwise be + /// this call will return failure if the mutex would otherwise be /// acquired. - pub fn try_lock(&self) -> Option> { - unsafe { - let lock: &'static StaticMutex = &*(&*self.inner as *const _); - lock.try_lock().map(|guard| { - MutexGuard::new(self, guard) - }) + pub fn try_lock(&self) -> TryLockResult> { + if unsafe { self.inner.lock.try_lock() } { + Ok(try!(MutexGuard::new(self))) + } else { + Err(TryLockError::WouldBlock) } } } @@ -196,17 +240,19 @@ impl Drop for Mutex { impl StaticMutex { /// Acquires this lock, see `Mutex::lock` - pub fn lock(&'static self) -> StaticMutexGuard { + #[inline] + pub fn lock(&'static self) -> LockResult { unsafe { self.lock.lock() } StaticMutexGuard::new(self) } /// Attempts to grab this lock, see `Mutex::try_lock` - pub fn try_lock(&'static self) -> Option { + #[inline] + pub fn try_lock(&'static self) -> TryLockResult { if unsafe { self.lock.try_lock() } { - Some(StaticMutexGuard::new(self)) + Ok(try!(StaticMutexGuard::new(self))) } else { - None + Err(TryLockError::WouldBlock) } } @@ -226,53 +272,85 @@ impl StaticMutex { } impl<'mutex, T> MutexGuard<'mutex, T> { - fn new(lock: &Mutex, guard: StaticMutexGuard) -> MutexGuard { - MutexGuard { __lock: lock, __guard: guard } + fn new(lock: &Mutex) -> LockResult> { + poison::map_result(Guard::new(lock), |guard| { + MutexGuard { __inner: guard } + }) } } -impl<'mutex, T> AsMutexGuard for MutexGuard<'mutex, T> { - unsafe fn as_mutex_guard(&self) -> &StaticMutexGuard { &self.__guard } +impl AsGuard for Mutex { + fn as_guard(&self) -> CondvarGuard { self.inner.as_guard() } } -impl<'mutex, T> Deref for MutexGuard<'mutex, T> { - fn deref<'a>(&'a self) -> &'a T { unsafe { &*self.__lock.data.get() } } -} -impl<'mutex, T> DerefMut for MutexGuard<'mutex, T> { - fn deref_mut<'a>(&'a mut self) -> &'a mut T { - unsafe { &mut *self.__lock.data.get() } - } -} - -impl StaticMutexGuard { - fn new(lock: &'static StaticMutex) -> StaticMutexGuard { - unsafe { - let guard = StaticMutexGuard { - lock: &lock.lock, - marker: marker::NoSend, - poison: (*lock.poison.get()).borrow(), - }; - guard.poison.check("mutex"); - return guard; +impl<'mutex, T> AsGuard for MutexGuard<'mutex, T> { + fn as_guard(&self) -> CondvarGuard { + CondvarGuard { + lock: &self.__inner.inner.inner.lock, + poisoned: &self.__inner.inner.inner.poison, } } } -pub fn guard_lock(guard: &StaticMutexGuard) -> &sys::Mutex { guard.lock } -pub fn guard_poison(guard: &StaticMutexGuard) -> &poison::Guard { - &guard.poison +impl<'mutex, T> Deref for MutexGuard<'mutex, T> { + fn deref<'a>(&'a self) -> &'a T { + unsafe { &*self.__inner.inner.data.get() } + } +} +impl<'mutex, T> DerefMut for MutexGuard<'mutex, T> { + fn deref_mut<'a>(&'a mut self) -> &'a mut T { + unsafe { &mut *self.__inner.inner.data.get() } + } } -impl AsMutexGuard for StaticMutexGuard { - unsafe fn as_mutex_guard(&self) -> &StaticMutexGuard { self } +impl StaticMutexGuard { + #[inline] + fn new(lock: &'static StaticMutex) -> LockResult { + poison::map_result(Guard::new(lock), |guard| { + StaticMutexGuard { inner: guard } + }) + } +} + +impl AsGuard for StaticMutex { + #[inline] + fn as_guard(&self) -> CondvarGuard { + CondvarGuard { lock: &self.lock, poisoned: &self.poison } + } +} + +impl AsGuard for StaticMutexGuard { + #[inline] + fn as_guard(&self) -> CondvarGuard { + CondvarGuard { + lock: &self.inner.inner.lock, + poisoned: &self.inner.inner.poison, + } + } +} + +impl<'a, T: AsGuard> Guard<'a, T> { + #[inline] + fn new(t: &T) -> LockResult> { + let data = t.as_guard(); + poison::map_result(data.poisoned.borrow(), |guard| { + Guard { + inner: t, + poison: guard, + marker: marker::NoSend, + } + }) + } } #[unsafe_destructor] -impl Drop for StaticMutexGuard { +impl<'a, T: AsGuard> Drop for Guard<'a, T> { + #[inline] fn drop(&mut self) { unsafe { - self.poison.done(); - self.lock.unlock(); + let data = self.inner.as_guard(); + data.poisoned.done(&self.poison); + data.lock.unlock(); } } } @@ -292,16 +370,16 @@ mod test { #[test] fn smoke() { let m = Mutex::new(()); - drop(m.lock()); - drop(m.lock()); + drop(m.lock().unwrap()); + drop(m.lock().unwrap()); } #[test] fn smoke_static() { static M: StaticMutex = MUTEX_INIT; unsafe { - drop(M.lock()); - drop(M.lock()); + drop(M.lock().unwrap()); + drop(M.lock().unwrap()); M.destroy(); } } @@ -316,7 +394,7 @@ mod test { fn inc() { for _ in range(0, J) { unsafe { - let _g = M.lock(); + let _g = M.lock().unwrap(); CNT += 1; } } @@ -343,7 +421,7 @@ mod test { #[test] fn try_lock() { let m = Mutex::new(()); - assert!(m.try_lock().is_some()); + *m.try_lock().unwrap() = (); } #[test] @@ -355,22 +433,21 @@ mod test { // wait until parent gets in rx.recv(); let &(ref lock, ref cvar) = &*packet2.0; - let mut lock = lock.lock(); + let mut lock = lock.lock().unwrap(); *lock = true; cvar.notify_one(); }); let &(ref lock, ref cvar) = &*packet.0; - let lock = lock.lock(); + let mut lock = lock.lock().unwrap(); tx.send(()); assert!(!*lock); while !*lock { - cvar.wait(&lock); + lock = cvar.wait(lock).unwrap(); } } #[test] - #[should_fail] fn test_arc_condvar_poison() { let packet = Packet(Arc::new((Mutex::new(1i), Condvar::new()))); let packet2 = Packet(packet.0.clone()); @@ -379,31 +456,35 @@ mod test { spawn(move|| { rx.recv(); let &(ref lock, ref cvar) = &*packet2.0; - let _g = lock.lock(); + let _g = lock.lock().unwrap(); cvar.notify_one(); // Parent should fail when it wakes up. panic!(); }); let &(ref lock, ref cvar) = &*packet.0; - let lock = lock.lock(); + let mut lock = lock.lock().unwrap(); tx.send(()); while *lock == 1 { - cvar.wait(&lock); + match cvar.wait(lock) { + Ok(l) => { + lock = l; + assert_eq!(*lock, 1); + } + Err(..) => break, + } } } #[test] - #[should_fail] fn test_mutex_arc_poison() { let arc = Arc::new(Mutex::new(1i)); let arc2 = arc.clone(); - let _ = Thread::spawn(move|| { - let lock = arc2.lock(); + Thread::spawn(move|| { + let lock = arc2.lock().unwrap(); assert_eq!(*lock, 2); }).join(); - let lock = arc.lock(); - assert_eq!(*lock, 1); + assert!(arc.lock().is_err()); } #[test] @@ -414,8 +495,8 @@ mod test { let arc2 = Arc::new(Mutex::new(arc)); let (tx, rx) = channel(); spawn(move|| { - let lock = arc2.lock(); - let lock2 = lock.deref().lock(); + let lock = arc2.lock().unwrap(); + let lock2 = lock.deref().lock().unwrap(); assert_eq!(*lock2, 1); tx.send(()); }); @@ -432,13 +513,13 @@ mod test { } impl Drop for Unwinder { fn drop(&mut self) { - *self.i.lock() += 1; + *self.i.lock().unwrap() += 1; } } let _u = Unwinder { i: arc2 }; panic!(); }).join(); - let lock = arc.lock(); + let lock = arc.lock().unwrap(); assert_eq!(*lock, 2); } } diff --git a/src/libstd/sync/poison.rs b/src/libstd/sync/poison.rs index ad08e9873fa..d99fd91d0ac 100644 --- a/src/libstd/sync/poison.rs +++ b/src/libstd/sync/poison.rs @@ -8,31 +8,120 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use prelude::*; + +use cell::UnsafeCell; +use error::FromError; +use fmt; use thread::Thread; -pub struct Flag { pub failed: bool } +pub struct Flag { failed: UnsafeCell } +pub const FLAG_INIT: Flag = Flag { failed: UnsafeCell { value: false } }; impl Flag { - pub fn borrow(&mut self) -> Guard { - Guard { flag: &mut self.failed, panicking: Thread::panicking() } + #[inline] + pub fn borrow(&self) -> LockResult { + let ret = Guard { panicking: Thread::panicking() }; + if unsafe { *self.failed.get() } { + Err(new_poison_error(ret)) + } else { + Ok(ret) + } + } + + #[inline] + pub fn done(&self, guard: &Guard) { + if !guard.panicking && Thread::panicking() { + unsafe { *self.failed.get() = true; } + } + } + + #[inline] + pub fn get(&self) -> bool { + unsafe { *self.failed.get() } } } -pub struct Guard<'a> { - flag: &'a mut bool, +#[allow(missing_copy_implementations)] +pub struct Guard { panicking: bool, } -impl<'a> Guard<'a> { - pub fn check(&self, name: &str) { - if *self.flag { - panic!("poisoned {} - another task failed inside", name); - } - } +/// A type of error which can be returned whenever a lock is acquired. +/// +/// Both Mutexes and RWLocks are poisoned whenever a task fails while the lock +/// is held. The precise semantics for when a lock is poisoned is documented on +/// each lock, but once a lock is poisoned then all future acquisitions will +/// return this error. +pub struct PoisonError { + guard: T, +} - pub fn done(&mut self) { - if !self.panicking && Thread::panicking() { - *self.flag = true; +/// An enumeration of possible errors which can occur while calling the +/// `try_lock` method. +pub enum TryLockError { + /// The lock could not be acquired because another task failed while holding + /// the lock. + Poisoned(PoisonError), + /// The lock could not be acquired at this time because the operation would + /// otherwise block. + WouldBlock, +} + +/// A type alias for the result of a lock method which can be poisoned. +/// +/// The `Ok` variant of this result indicates that the primitive was not +/// poisoned, and the `Guard` is contained within. The `Err` variant indicates +/// that the primitive was poisoned. Note that the `Err` variant *also* carries +/// the associated guard, and it can be acquired through the `into_inner` +/// method. +pub type LockResult = Result>; + +/// A type alias for the result of a nonblocking locking method. +/// +/// For more information, see `LockResult`. A `TryLockResult` doesn't +/// necessarily hold the associated guard in the `Err` type as the lock may not +/// have been acquired for other reasons. +pub type TryLockResult = Result>; + +impl fmt::Show for PoisonError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + "poisoned lock: another task failed inside".fmt(f) + } +} + +impl PoisonError { + /// Consumes this error indicating that a lock is poisoned, returning the + /// underlying guard to allow access regardless. + pub fn into_guard(self) -> T { self.guard } +} + +impl FromError> for TryLockError { + fn from_error(err: PoisonError) -> TryLockError { + TryLockError::Poisoned(err) + } +} + +impl fmt::Show for TryLockError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self { + TryLockError::Poisoned(ref p) => p.fmt(f), + TryLockError::WouldBlock => { + "try_lock failed because the operation would block".fmt(f) + } } } } + +pub fn new_poison_error(guard: T) -> PoisonError { + PoisonError { guard: guard } +} + +pub fn map_result(result: LockResult, f: F) + -> LockResult + where F: FnOnce(T) -> U { + match result { + Ok(t) => Ok(f(t)), + Err(PoisonError { guard }) => Err(new_poison_error(f(guard))) + } +} diff --git a/src/libstd/sync/rwlock.rs b/src/libstd/sync/rwlock.rs index 76d05d9bfd4..f7632c4f8b5 100644 --- a/src/libstd/sync/rwlock.rs +++ b/src/libstd/sync/rwlock.rs @@ -10,10 +10,10 @@ use prelude::*; -use kinds::marker; use cell::UnsafeCell; +use kinds::marker; +use sync::poison::{mod, LockResult, TryLockError, TryLockResult}; use sys_common::rwlock as sys; -use sync::poison; /// A reader-writer lock /// @@ -28,12 +28,14 @@ use sync::poison; /// locking methods implement `Deref` (and `DerefMut` for the `write` methods) /// to allow access to the contained of the lock. /// +/// # Poisoning +/// /// RWLocks, like Mutexes, will become poisoned on panics. Note, however, that /// an RWLock may only be poisoned if a panic occurs while it is locked /// exclusively (write mode). If a panic occurs in any reader, then the lock /// will not be poisoned. /// -/// # Example +/// # Examples /// /// ``` /// use std::sync::RWLock; @@ -42,15 +44,15 @@ use sync::poison; /// /// // many reader locks can be held at once /// { -/// let r1 = lock.read(); -/// let r2 = lock.read(); +/// let r1 = lock.read().unwrap(); +/// let r2 = lock.read().unwrap(); /// assert_eq!(*r1, 5); /// assert_eq!(*r2, 5); /// } // read locks are dropped at this point /// /// // only one write lock may be held, however /// { -/// let mut w = lock.write(); +/// let mut w = lock.write().unwrap(); /// *w += 1; /// assert_eq!(*w, 6); /// } // write lock is dropped here @@ -77,18 +79,18 @@ unsafe impl Sync for RWLock {} /// static LOCK: StaticRWLock = RWLOCK_INIT; /// /// { -/// let _g = LOCK.read(); +/// let _g = LOCK.read().unwrap(); /// // ... shared read access /// } /// { -/// let _g = LOCK.write(); +/// let _g = LOCK.write().unwrap(); /// // ... exclusive write access /// } /// unsafe { LOCK.destroy() } // free all resources /// ``` pub struct StaticRWLock { - inner: sys::RWLock, - poison: UnsafeCell, + lock: sys::RWLock, + poison: poison::Flag, } unsafe impl Send for StaticRWLock {} @@ -96,41 +98,52 @@ unsafe impl Sync for StaticRWLock {} /// Constant initialization for a statically-initialized rwlock. pub const RWLOCK_INIT: StaticRWLock = StaticRWLock { - inner: sys::RWLOCK_INIT, - poison: UnsafeCell { value: poison::Flag { failed: false } }, + lock: sys::RWLOCK_INIT, + poison: poison::FLAG_INIT, }; /// RAII structure used to release the shared read access of a lock when /// dropped. #[must_use] pub struct RWLockReadGuard<'a, T: 'a> { - __lock: &'a RWLock, - __guard: StaticRWLockReadGuard, + __inner: ReadGuard<'a, RWLock>, } /// RAII structure used to release the exclusive write access of a lock when /// dropped. #[must_use] pub struct RWLockWriteGuard<'a, T: 'a> { - __lock: &'a RWLock, - __guard: StaticRWLockWriteGuard, + __inner: WriteGuard<'a, RWLock>, } /// RAII structure used to release the shared read access of a lock when /// dropped. #[must_use] pub struct StaticRWLockReadGuard { - lock: &'static sys::RWLock, - marker: marker::NoSend, + _inner: ReadGuard<'static, StaticRWLock>, } /// RAII structure used to release the exclusive write access of a lock when /// dropped. #[must_use] pub struct StaticRWLockWriteGuard { - lock: &'static sys::RWLock, - marker: marker::NoSend, - poison: poison::Guard<'static>, + _inner: WriteGuard<'static, StaticRWLock>, +} + +struct ReadGuard<'a, T: 'a> { + inner: &'a T, + marker: marker::NoSend, // even if 'a == static, cannot send +} + +struct WriteGuard<'a, T: 'a> { + inner: &'a T, + poison: poison::Guard, + marker: marker::NoSend, // even if 'a == static, cannot send +} + +#[doc(hidden)] +trait AsStaticRWLock { + fn as_static_rwlock(&self) -> &StaticRWLock; } impl RWLock { @@ -151,17 +164,15 @@ impl RWLock { /// Returns an RAII guard which will release this thread's shared access /// once it is dropped. /// - /// # Panics + /// # Failure /// - /// This function will panic if the RWLock is poisoned. An RWLock is - /// poisoned whenever a writer panics while holding an exclusive lock. The - /// panic will occur immediately after the lock has been acquired. + /// This function will return an error if the RWLock is poisoned. An RWLock + /// is poisoned whenever a writer panics while holding an exclusive lock. + /// The failure will occur immediately after the lock has been acquired. #[inline] - pub fn read(&self) -> RWLockReadGuard { - unsafe { - let lock: &'static StaticRWLock = &*(&*self.inner as *const _); - RWLockReadGuard::new(self, lock.read()) - } + pub fn read(&self) -> LockResult> { + unsafe { self.inner.lock.read() } + RWLockReadGuard::new(self) } /// Attempt to acquire this lock with shared read access. @@ -173,18 +184,18 @@ impl RWLock { /// guarantees with respect to the ordering of whether contentious readers /// or writers will acquire the lock first. /// - /// # Panics + /// # Failure /// - /// This function will panic if the RWLock is poisoned. An RWLock is - /// poisoned whenever a writer panics while holding an exclusive lock. A - /// panic will only occur if the lock is acquired. + /// This function will return an error if the RWLock is poisoned. An RWLock + /// is poisoned whenever a writer panics while holding an exclusive lock. An + /// error will only be returned if the lock would have otherwise been + /// acquired. #[inline] - pub fn try_read(&self) -> Option> { - unsafe { - let lock: &'static StaticRWLock = &*(&*self.inner as *const _); - lock.try_read().map(|guard| { - RWLockReadGuard::new(self, guard) - }) + pub fn try_read(&self) -> TryLockResult> { + if unsafe { self.inner.lock.try_read() } { + Ok(try!(RWLockReadGuard::new(self))) + } else { + Err(TryLockError::WouldBlock) } } @@ -197,17 +208,15 @@ impl RWLock { /// Returns an RAII guard which will drop the write access of this rwlock /// when dropped. /// - /// # Panics + /// # Failure /// - /// This function will panic if the RWLock is poisoned. An RWLock is - /// poisoned whenever a writer panics while holding an exclusive lock. The - /// panic will occur when the lock is acquired. + /// This function will return an error if the RWLock is poisoned. An RWLock + /// is poisoned whenever a writer panics while holding an exclusive lock. + /// An error will be returned when the lock is acquired. #[inline] - pub fn write(&self) -> RWLockWriteGuard { - unsafe { - let lock: &'static StaticRWLock = &*(&*self.inner as *const _); - RWLockWriteGuard::new(self, lock.write()) - } + pub fn write(&self) -> LockResult> { + unsafe { self.inner.lock.write() } + RWLockWriteGuard::new(self) } /// Attempt to lock this rwlock with exclusive write access. @@ -216,18 +225,18 @@ impl RWLock { /// to `write` would otherwise block. If successful, an RAII guard is /// returned. /// - /// # Panics + /// # Failure /// - /// This function will panic if the RWLock is poisoned. An RWLock is - /// poisoned whenever a writer panics while holding an exclusive lock. A - /// panic will only occur if the lock is acquired. + /// This function will return an error if the RWLock is poisoned. An RWLock + /// is poisoned whenever a writer panics while holding an exclusive lock. An + /// error will only be returned if the lock would have otherwise been + /// acquired. #[inline] - pub fn try_write(&self) -> Option> { - unsafe { - let lock: &'static StaticRWLock = &*(&*self.inner as *const _); - lock.try_write().map(|guard| { - RWLockWriteGuard::new(self, guard) - }) + pub fn try_write(&self) -> TryLockResult> { + if unsafe { self.inner.lock.try_read() } { + Ok(try!(RWLockWriteGuard::new(self))) + } else { + Err(TryLockError::WouldBlock) } } } @@ -235,7 +244,7 @@ impl RWLock { #[unsafe_destructor] impl Drop for RWLock { fn drop(&mut self) { - unsafe { self.inner.inner.destroy() } + unsafe { self.inner.lock.destroy() } } } @@ -245,8 +254,8 @@ impl StaticRWLock { /// /// See `RWLock::read`. #[inline] - pub fn read(&'static self) -> StaticRWLockReadGuard { - unsafe { self.inner.read() } + pub fn read(&'static self) -> LockResult { + unsafe { self.lock.read() } StaticRWLockReadGuard::new(self) } @@ -254,11 +263,11 @@ impl StaticRWLock { /// /// See `RWLock::try_read`. #[inline] - pub fn try_read(&'static self) -> Option { - if unsafe { self.inner.try_read() } { - Some(StaticRWLockReadGuard::new(self)) + pub fn try_read(&'static self) -> TryLockResult { + if unsafe { self.lock.try_read() } { + Ok(try!(StaticRWLockReadGuard::new(self))) } else { - None + Err(TryLockError::WouldBlock) } } @@ -267,8 +276,8 @@ impl StaticRWLock { /// /// See `RWLock::write`. #[inline] - pub fn write(&'static self) -> StaticRWLockWriteGuard { - unsafe { self.inner.write() } + pub fn write(&'static self) -> LockResult { + unsafe { self.lock.write() } StaticRWLockWriteGuard::new(self) } @@ -276,11 +285,11 @@ impl StaticRWLock { /// /// See `RWLock::try_write`. #[inline] - pub fn try_write(&'static self) -> Option { - if unsafe { self.inner.try_write() } { - Some(StaticRWLockWriteGuard::new(self)) + pub fn try_write(&'static self) -> TryLockResult { + if unsafe { self.lock.try_write() } { + Ok(try!(StaticRWLockWriteGuard::new(self))) } else { - None + Err(TryLockError::WouldBlock) } } @@ -291,69 +300,92 @@ impl StaticRWLock { /// of this lock. This method is required to be called to not leak memory on /// all platforms. pub unsafe fn destroy(&'static self) { - self.inner.destroy() + self.lock.destroy() } } impl<'rwlock, T> RWLockReadGuard<'rwlock, T> { - fn new(lock: &RWLock, guard: StaticRWLockReadGuard) - -> RWLockReadGuard { - RWLockReadGuard { __lock: lock, __guard: guard } + fn new(lock: &RWLock) -> LockResult> { + poison::map_result(ReadGuard::new(lock), |guard| { + RWLockReadGuard { __inner: guard } + }) } } impl<'rwlock, T> RWLockWriteGuard<'rwlock, T> { - fn new(lock: &RWLock, guard: StaticRWLockWriteGuard) - -> RWLockWriteGuard { - RWLockWriteGuard { __lock: lock, __guard: guard } + fn new(lock: &RWLock) -> LockResult> { + poison::map_result(WriteGuard::new(lock), |guard| { + RWLockWriteGuard { __inner: guard } + }) } } impl<'rwlock, T> Deref for RWLockReadGuard<'rwlock, T> { - fn deref(&self) -> &T { unsafe { &*self.__lock.data.get() } } + fn deref(&self) -> &T { unsafe { &*self.__inner.inner.data.get() } } } impl<'rwlock, T> Deref for RWLockWriteGuard<'rwlock, T> { - fn deref(&self) -> &T { unsafe { &*self.__lock.data.get() } } + fn deref(&self) -> &T { unsafe { &*self.__inner.inner.data.get() } } } impl<'rwlock, T> DerefMut for RWLockWriteGuard<'rwlock, T> { - fn deref_mut(&mut self) -> &mut T { unsafe { &mut *self.__lock.data.get() } } + fn deref_mut(&mut self) -> &mut T { + unsafe { &mut *self.__inner.inner.data.get() } + } } impl StaticRWLockReadGuard { - fn new(lock: &'static StaticRWLock) -> StaticRWLockReadGuard { - let guard = StaticRWLockReadGuard { - lock: &lock.inner, - marker: marker::NoSend, - }; - unsafe { (*lock.poison.get()).borrow().check("rwlock"); } - return guard; + #[inline] + fn new(lock: &'static StaticRWLock) -> LockResult { + poison::map_result(ReadGuard::new(lock), |guard| { + StaticRWLockReadGuard { _inner: guard } + }) } } impl StaticRWLockWriteGuard { - fn new(lock: &'static StaticRWLock) -> StaticRWLockWriteGuard { - unsafe { - let guard = StaticRWLockWriteGuard { - lock: &lock.inner, - marker: marker::NoSend, - poison: (*lock.poison.get()).borrow(), - }; - guard.poison.check("rwlock"); - return guard; - } + #[inline] + fn new(lock: &'static StaticRWLock) -> LockResult { + poison::map_result(WriteGuard::new(lock), |guard| { + StaticRWLockWriteGuard { _inner: guard } + }) + } +} + +impl AsStaticRWLock for RWLock { + #[inline] + fn as_static_rwlock(&self) -> &StaticRWLock { &*self.inner } +} +impl AsStaticRWLock for StaticRWLock { + #[inline] + fn as_static_rwlock(&self) -> &StaticRWLock { self } +} + +impl<'a, T: AsStaticRWLock> ReadGuard<'a, T> { + fn new(t: &'a T) -> LockResult> { + poison::map_result(t.as_static_rwlock().poison.borrow(), |_| { + ReadGuard { inner: t, marker: marker::NoSend } + }) + } +} + +impl<'a, T: AsStaticRWLock> WriteGuard<'a, T> { + fn new(t: &'a T) -> LockResult> { + poison::map_result(t.as_static_rwlock().poison.borrow(), |guard| { + WriteGuard { inner: t, marker: marker::NoSend, poison: guard } + }) } } #[unsafe_destructor] -impl Drop for StaticRWLockReadGuard { +impl<'a, T: AsStaticRWLock> Drop for ReadGuard<'a, T> { fn drop(&mut self) { - unsafe { self.lock.read_unlock(); } + unsafe { self.inner.as_static_rwlock().lock.read_unlock(); } } } #[unsafe_destructor] -impl Drop for StaticRWLockWriteGuard { +impl<'a, T: AsStaticRWLock> Drop for WriteGuard<'a, T> { fn drop(&mut self) { - self.poison.done(); - unsafe { self.lock.write_unlock(); } + let inner = self.inner.as_static_rwlock(); + inner.poison.done(&self.poison); + unsafe { inner.lock.write_unlock(); } } } @@ -368,19 +400,19 @@ mod tests { #[test] fn smoke() { let l = RWLock::new(()); - drop(l.read()); - drop(l.write()); - drop((l.read(), l.read())); - drop(l.write()); + drop(l.read().unwrap()); + drop(l.write().unwrap()); + drop((l.read().unwrap(), l.read().unwrap())); + drop(l.write().unwrap()); } #[test] fn static_smoke() { static R: StaticRWLock = RWLOCK_INIT; - drop(R.read()); - drop(R.write()); - drop((R.read(), R.read())); - drop(R.write()); + drop(R.read().unwrap()); + drop(R.write().unwrap()); + drop((R.read().unwrap(), R.read().unwrap())); + drop(R.write().unwrap()); unsafe { R.destroy(); } } @@ -397,9 +429,9 @@ mod tests { let mut rng = rand::task_rng(); for _ in range(0, M) { if rng.gen_weighted_bool(N) { - drop(R.write()); + drop(R.write().unwrap()); } else { - drop(R.read()); + drop(R.read().unwrap()); } } drop(tx); @@ -411,51 +443,47 @@ mod tests { } #[test] - #[should_fail] fn test_rw_arc_poison_wr() { let arc = Arc::new(RWLock::new(1i)); let arc2 = arc.clone(); - let _ = Thread::spawn(move|| { - let lock = arc2.write(); - assert_eq!(*lock, 2); + let _: Result = Thread::spawn(move|| { + let _lock = arc2.write().unwrap(); + panic!(); }).join(); - let lock = arc.read(); - assert_eq!(*lock, 1); + assert!(arc.read().is_err()); } #[test] - #[should_fail] fn test_rw_arc_poison_ww() { let arc = Arc::new(RWLock::new(1i)); let arc2 = arc.clone(); - let _ = Thread::spawn(move|| { - let lock = arc2.write(); - assert_eq!(*lock, 2); + let _: Result = Thread::spawn(move|| { + let _lock = arc2.write().unwrap(); + panic!(); }).join(); - let lock = arc.write(); - assert_eq!(*lock, 1); + assert!(arc.write().is_err()); } #[test] fn test_rw_arc_no_poison_rr() { let arc = Arc::new(RWLock::new(1i)); let arc2 = arc.clone(); - let _ = Thread::spawn(move|| { - let lock = arc2.read(); - assert_eq!(*lock, 2); + let _: Result = Thread::spawn(move|| { + let _lock = arc2.read().unwrap(); + panic!(); }).join(); - let lock = arc.read(); + let lock = arc.read().unwrap(); assert_eq!(*lock, 1); } #[test] fn test_rw_arc_no_poison_rw() { let arc = Arc::new(RWLock::new(1i)); let arc2 = arc.clone(); - let _ = Thread::spawn(move|| { - let lock = arc2.read(); - assert_eq!(*lock, 2); + let _: Result = Thread::spawn(move|| { + let _lock = arc2.read().unwrap(); + panic!() }).join(); - let lock = arc.write(); + let lock = arc.write().unwrap(); assert_eq!(*lock, 1); } @@ -466,7 +494,7 @@ mod tests { let (tx, rx) = channel(); Thread::spawn(move|| { - let mut lock = arc2.write(); + let mut lock = arc2.write().unwrap(); for _ in range(0u, 10) { let tmp = *lock; *lock = -1; @@ -481,7 +509,7 @@ mod tests { for _ in range(0u, 5) { let arc3 = arc.clone(); children.push(Thread::spawn(move|| { - let lock = arc3.read(); + let lock = arc3.read().unwrap(); assert!(*lock >= 0); })); } @@ -493,7 +521,7 @@ mod tests { // Wait for writer to finish rx.recv(); - let lock = arc.read(); + let lock = arc.read().unwrap(); assert_eq!(*lock, 10); } @@ -507,14 +535,14 @@ mod tests { } impl Drop for Unwinder { fn drop(&mut self) { - let mut lock = self.i.write(); + let mut lock = self.i.write().unwrap(); *lock += 1; } } let _u = Unwinder { i: arc2 }; panic!(); }).join(); - let lock = arc.read(); + let lock = arc.read().unwrap(); assert_eq!(*lock, 2); } } diff --git a/src/libstd/sync/semaphore.rs b/src/libstd/sync/semaphore.rs index 574b0f22bee..e3b683a6ccb 100644 --- a/src/libstd/sync/semaphore.rs +++ b/src/libstd/sync/semaphore.rs @@ -68,9 +68,9 @@ impl Semaphore { /// This method will block until the internal count of the semaphore is at /// least 1. pub fn acquire(&self) { - let mut count = self.lock.lock(); + let mut count = self.lock.lock().unwrap(); while *count <= 0 { - self.cvar.wait(&count); + count = self.cvar.wait(count).unwrap(); } *count -= 1; } @@ -80,7 +80,7 @@ impl Semaphore { /// This will increment the number of resources in this semaphore by 1 and /// will notify any pending waiters in `acquire` or `access` if necessary. pub fn release(&self) { - *self.lock.lock() += 1; + *self.lock.lock().unwrap() += 1; self.cvar.notify_one(); } diff --git a/src/libstd/sync/task_pool.rs b/src/libstd/sync/task_pool.rs index 366e4b7d35b..98da5ccc554 100644 --- a/src/libstd/sync/task_pool.rs +++ b/src/libstd/sync/task_pool.rs @@ -113,7 +113,7 @@ fn spawn_in_pool(jobs: Arc>>) { let message = { // Only lock jobs for the time it takes // to get a job, not run it. - let lock = jobs.lock(); + let lock = jobs.lock().unwrap(); lock.recv_opt() }; diff --git a/src/libstd/sys/common/helper_thread.rs b/src/libstd/sys/common/helper_thread.rs index a629f035b07..9ef1c33312f 100644 --- a/src/libstd/sys/common/helper_thread.rs +++ b/src/libstd/sys/common/helper_thread.rs @@ -83,7 +83,7 @@ impl Helper { F: FnOnce() -> T, { unsafe { - let _guard = self.lock.lock(); + let _guard = self.lock.lock().unwrap(); if !*self.initialized.get() { let (tx, rx) = channel(); *self.chan.get() = mem::transmute(box tx); @@ -95,7 +95,7 @@ impl Helper { let t = f(); Thread::spawn(move |:| { helper(receive.0, rx, t); - let _g = self.lock.lock(); + let _g = self.lock.lock().unwrap(); *self.shutdown.get() = true; self.cond.notify_one() }).detach(); @@ -111,7 +111,7 @@ impl Helper { /// This is only valid if the worker thread has previously booted pub fn send(&'static self, msg: M) { unsafe { - let _guard = self.lock.lock(); + let _guard = self.lock.lock().unwrap(); // Must send and *then* signal to ensure that the child receives the // message. Otherwise it could wake up and go to sleep before we @@ -127,7 +127,7 @@ impl Helper { // Shut down, but make sure this is done inside our lock to ensure // that we'll always receive the exit signal when the thread // returns. - let guard = self.lock.lock(); + let mut guard = self.lock.lock().unwrap(); // Close the channel by destroying it let chan: Box> = mem::transmute(*self.chan.get()); @@ -137,7 +137,7 @@ impl Helper { // Wait for the child to exit while !*self.shutdown.get() { - self.cond.wait(&guard); + guard = self.cond.wait(guard).unwrap(); } drop(guard); diff --git a/src/libstd/thread.rs b/src/libstd/thread.rs index 56731bd7ec3..38e304ccfec 100644 --- a/src/libstd/thread.rs +++ b/src/libstd/thread.rs @@ -334,6 +334,7 @@ impl Thread { } /// Determines whether the current thread is panicking. + #[inline] pub fn panicking() -> bool { unwind::panicking() } @@ -349,9 +350,9 @@ impl Thread { // or futuxes, and in either case may allow spurious wakeups. pub fn park() { let thread = Thread::current(); - let mut guard = thread.inner.lock.lock(); + let mut guard = thread.inner.lock.lock().unwrap(); while !*guard { - thread.inner.cvar.wait(&guard); + guard = thread.inner.cvar.wait(guard).unwrap(); } *guard = false; } @@ -360,7 +361,7 @@ impl Thread { /// /// See the module doc for more detail. pub fn unpark(&self) { - let mut guard = self.inner.lock.lock(); + let mut guard = self.inner.lock.lock().unwrap(); if !*guard { *guard = true; self.inner.cvar.notify_one(); diff --git a/src/libstd/thread_local/mod.rs b/src/libstd/thread_local/mod.rs index 242dceb4256..4cfa2709352 100644 --- a/src/libstd/thread_local/mod.rs +++ b/src/libstd/thread_local/mod.rs @@ -240,13 +240,18 @@ impl Key { unsafe { let slot = slot.get().expect("cannot access a TLS value during or \ after it is destroyed"); - if (*slot.get()).is_none() { - *slot.get() = Some((self.init)()); - } - f((*slot.get()).as_ref().unwrap()) + f(match *slot.get() { + Some(ref inner) => inner, + None => self.init(slot), + }) } } + unsafe fn init(&self, slot: &UnsafeCell>) -> &T { + *slot.get() = Some((self.init)()); + (*slot.get()).as_ref().unwrap() + } + /// Test this TLS key to determine whether its value has been destroyed for /// the current thread or not. /// diff --git a/src/test/bench/msgsend-ring-mutex-arcs.rs b/src/test/bench/msgsend-ring-mutex-arcs.rs index 49f53bf9d38..8ec44b2dd3c 100644 --- a/src/test/bench/msgsend-ring-mutex-arcs.rs +++ b/src/test/bench/msgsend-ring-mutex-arcs.rs @@ -28,15 +28,15 @@ type pipe = Arc<(Mutex>, Condvar)>; fn send(p: &pipe, msg: uint) { let &(ref lock, ref cond) = &**p; - let mut arr = lock.lock(); + let mut arr = lock.lock().unwrap(); arr.push(msg); cond.notify_one(); } fn recv(p: &pipe) -> uint { let &(ref lock, ref cond) = &**p; - let mut arr = lock.lock(); + let mut arr = lock.lock().unwrap(); while arr.is_empty() { - cond.wait(&arr); + arr = cond.wait(arr).unwrap(); } arr.pop().unwrap() } diff --git a/src/test/run-pass/match-ref-binding-in-guard-3256.rs b/src/test/run-pass/match-ref-binding-in-guard-3256.rs index 9bb912e081c..a83bc73457e 100644 --- a/src/test/run-pass/match-ref-binding-in-guard-3256.rs +++ b/src/test/run-pass/match-ref-binding-in-guard-3256.rs @@ -11,13 +11,11 @@ use std::sync::Mutex; pub fn main() { - unsafe { - let x = Some(Mutex::new(true)); - match x { - Some(ref z) if *z.lock() => { - assert!(*z.lock()); - }, - _ => panic!() - } + let x = Some(Mutex::new(true)); + match x { + Some(ref z) if *z.lock().unwrap() => { + assert!(*z.lock().unwrap()); + }, + _ => panic!() } } diff --git a/src/test/run-pass/writealias.rs b/src/test/run-pass/writealias.rs index f48272366e2..dacfeb00819 100644 --- a/src/test/run-pass/writealias.rs +++ b/src/test/run-pass/writealias.rs @@ -15,13 +15,11 @@ struct Point {x: int, y: int, z: int} fn f(p: &mut Point) { p.z = 13; } pub fn main() { - unsafe { - let x = Some(Mutex::new(true)); - match x { - Some(ref z) if *z.lock() => { - assert!(*z.lock()); - }, - _ => panic!() - } + let x = Some(Mutex::new(true)); + match x { + Some(ref z) if *z.lock().unwrap() => { + assert!(*z.lock().unwrap()); + }, + _ => panic!() } }