From b8ffad5964e328340de0c03779577eb14caa16fc Mon Sep 17 00:00:00 2001 From: Steve Klabnik Date: Fri, 26 Dec 2014 16:04:27 -0500 Subject: s/task/thread/g A part of #20038 --- src/liballoc/rc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/liballoc') diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index dfa55848c90..e927cb72a13 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -//! Task-local reference-counted boxes (the `Rc` type). +//! Thread-local reference-counted boxes (the `Rc` type). //! //! The `Rc` type provides shared ownership of an immutable value. Destruction is deterministic, //! and will occur as soon as the last owner is gone. It is marked as non-sendable because it -- cgit 1.4.1-3-g733a5 From 76e5ed655c762b812c3da4749a55f1bb1b52c787 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 8 Dec 2014 20:20:03 -0800 Subject: std: Return Result from RWLock/Mutex methods All of the current std::sync primitives have poisoning enable which means that when a task fails inside of a write-access lock then all future attempts to acquire the lock will fail. This strategy ensures that stale data whose invariants are possibly not upheld are never viewed by other tasks to help propagate unexpected panics (bugs in a program) among tasks. Currently there is no way to test whether a mutex or rwlock is poisoned. One method would be to duplicate all the methods with a sister foo_catch function, for example. This pattern is, however, against our [error guidelines][errors]. As a result, this commit exposes the fact that a task has failed internally through the return value of a `Result`. [errors]: https://github.com/rust-lang/rfcs/blob/master/text/0236-error-conventions.md#do-not-provide-both-result-and-fail-variants All methods now return a `LockResult` or a `TryLockResult` which communicates whether the lock was poisoned or not. In a `LockResult`, both the `Ok` and `Err` variants contains the `MutexGuard` that is being returned in order to allow access to the data if poisoning is not desired. This also means that the lock is *always* held upon returning from `.lock()`. A new type, `PoisonError`, was added with one method `into_guard` which can consume the assertion that a lock is poisoned to gain access to the underlying data. This is a breaking change because the signatures of these methods have changed, often incompatible ways. One major difference is that the `wait` methods on a condition variable now consume the guard and return it in as a `LockResult` to indicate whether the lock was poisoned while waiting. Most code can be updated by calling `.unwrap()` on the return value of `.lock()`. [breaking-change] --- src/doc/intro.md | 2 +- src/liballoc/arc.rs | 4 +- src/librustc_trans/back/write.rs | 6 +- src/libstd/comm/shared.rs | 4 +- src/libstd/comm/sync.rs | 26 +- src/libstd/io/stdio.rs | 20 +- src/libstd/sync/barrier.rs | 4 +- src/libstd/sync/condvar.rs | 99 ++++--- src/libstd/sync/mod.rs | 15 +- src/libstd/sync/mutex.rs | 243 ++++++++++------ src/libstd/sync/poison.rs | 115 +++++++- src/libstd/sync/rwlock.rs | 306 +++++++++++---------- src/libstd/sync/semaphore.rs | 6 +- src/libstd/sync/task_pool.rs | 2 +- src/libstd/sys/common/helper_thread.rs | 10 +- src/libstd/thread.rs | 7 +- src/libstd/thread_local/mod.rs | 13 +- src/test/bench/msgsend-ring-mutex-arcs.rs | 6 +- .../run-pass/match-ref-binding-in-guard-3256.rs | 14 +- src/test/run-pass/writealias.rs | 14 +- 20 files changed, 572 insertions(+), 344 deletions(-) (limited to 'src/liballoc') 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> AsGuard for MutexGuard<'mutex, T> { + fn as_guard(&self) -> CondvarGuard { + CondvarGuard { + lock: &self.__inner.inner.inner.lock, + poisoned: &self.__inner.inner.inner.poison, + } + } } impl<'mutex, T> Deref for MutexGuard<'mutex, T> { - fn deref<'a>(&'a self) -> &'a T { unsafe { &*self.__lock.data.get() } } + 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.__lock.data.get() } + unsafe { &mut *self.__inner.inner.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; - } + #[inline] + fn new(lock: &'static StaticMutex) -> LockResult { + poison::map_result(Guard::new(lock), |guard| { + StaticMutexGuard { inner: guard } + }) } } -pub fn guard_lock(guard: &StaticMutexGuard) -> &sys::Mutex { guard.lock } -pub fn guard_poison(guard: &StaticMutexGuard) -> &poison::Guard { - &guard.poison +impl AsGuard for StaticMutex { + #[inline] + fn as_guard(&self) -> CondvarGuard { + CondvarGuard { lock: &self.lock, poisoned: &self.poison } + } } -impl AsMutexGuard for StaticMutexGuard { - unsafe fn as_mutex_guard(&self) -> &StaticMutexGuard { self } +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, +} + +/// 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) } +} - pub fn done(&mut self) { - if !self.panicking && Thread::panicking() { - *self.flag = true; +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!() } } -- cgit 1.4.1-3-g733a5 From 54452cdd68a18b7caa8def20bbd587b769f4cb67 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 19 Dec 2014 08:57:12 -0800 Subject: std: Second pass stabilization for `ptr` This commit performs a second pass for stabilization over the `std::ptr` module. The specific actions taken were: * The `RawPtr` trait was renamed to `PtrExt` * The `RawMutPtr` trait was renamed to `MutPtrExt` * The module name `ptr` is now stable. * These functions were all marked `#[stable]` with no modification: * `null` * `null_mut` * `swap` * `replace` * `read` * `write` * `PtrExt::is_null` * `PtrExt::offset` * These functions remain unstable: * `as_ref`, `as_mut` - the return value of an `Option` is not fully expressive as null isn't the only bad value, and it's unclear whether we want to commit to these functions at this time. The reference/lifetime semantics as written are also problematic in how they encourage arbitrary lifetimes. * `zero_memory` - This function is currently not used at all in the distribution, and in general it plays a broader role in the "working with unsafe pointers" story. This story is not yet fully developed, so at this time the function remains unstable for now. * `read_and_zero` - This function remains unstable for largely the same reasons as `zero_memory`. * These functions are now all deprecated: * `PtrExt::null` - call `ptr::null` or `ptr::null_mut` instead. * `PtrExt::to_uint` - use an `as` expression instead. * `PtrExt::is_not_null` - use `!p.is_null()` instead. --- src/liballoc/arc.rs | 2 +- src/liballoc/heap.rs | 4 +- src/liballoc/rc.rs | 2 +- src/libcollections/dlist.rs | 2 +- src/libcollections/slice.rs | 2 +- src/libcollections/str.rs | 17 ++--- src/libcore/prelude.rs | 2 +- src/libcore/ptr.rs | 141 +++++++++++++++++++++-------------- src/libcore/slice.rs | 4 +- src/libcore/str/mod.rs | 4 +- src/librustc_trans/trans/builder.rs | 4 +- src/librustc_trans/trans/value.rs | 10 +-- src/libstd/c_vec.rs | 2 +- src/libstd/collections/hash/table.rs | 2 +- src/libstd/io/extensions.rs | 2 +- src/libstd/io/mod.rs | 2 +- src/libstd/os.rs | 3 +- src/libstd/prelude.rs | 2 +- src/libstd/sys/common/net.rs | 2 +- src/libstd/sys/unix/backtrace.rs | 2 +- 20 files changed, 119 insertions(+), 92 deletions(-) (limited to 'src/liballoc') diff --git a/src/liballoc/arc.rs b/src/liballoc/arc.rs index 3e235caab18..eecc0769bd7 100644 --- a/src/liballoc/arc.rs +++ b/src/liballoc/arc.rs @@ -80,7 +80,7 @@ use core::nonzero::NonZero; use core::ops::{Drop, Deref}; use core::option::Option; use core::option::Option::{Some, None}; -use core::ptr::{mod, RawPtr}; +use core::ptr::{mod, PtrExt}; use heap::deallocate; /// An atomically reference counted wrapper for shared state. diff --git a/src/liballoc/heap.rs b/src/liballoc/heap.rs index c6b6a784f06..cdc30efd2d9 100644 --- a/src/liballoc/heap.rs +++ b/src/liballoc/heap.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use core::ptr::RawPtr; +use core::ptr::PtrExt; // FIXME: #13996: mark the `allocate` and `reallocate` return value as `noalias` @@ -371,7 +371,7 @@ mod imp { mod test { extern crate test; use self::test::Bencher; - use core::ptr::RawPtr; + use core::ptr::PtrExt; use heap; #[test] diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 13dc4474c1a..90e126bef4d 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -154,7 +154,7 @@ use core::nonzero::NonZero; use core::ops::{Deref, Drop}; use core::option::Option; use core::option::Option::{Some, None}; -use core::ptr::{mod, RawPtr}; +use core::ptr::{mod, PtrExt}; use core::result::Result; use core::result::Result::{Ok, Err}; diff --git a/src/libcollections/dlist.rs b/src/libcollections/dlist.rs index f20b37cb60f..d8ce79f4fe9 100644 --- a/src/libcollections/dlist.rs +++ b/src/libcollections/dlist.rs @@ -95,7 +95,7 @@ impl Rawlink { /// Convert the `Rawlink` into an Option value fn resolve_immut<'a>(&self) -> Option<&'a T> { unsafe { - self.p.as_ref() + mem::transmute(self.p.as_ref()) } } diff --git a/src/libcollections/slice.rs b/src/libcollections/slice.rs index d6d94f57acf..d4db9ea59f9 100644 --- a/src/libcollections/slice.rs +++ b/src/libcollections/slice.rs @@ -96,7 +96,7 @@ use core::mem::size_of; use core::mem; use core::ops::FnMut; use core::prelude::{Clone, Greater, Iterator, IteratorExt, Less, None, Option}; -use core::prelude::{Ord, Ordering, RawPtr, Some, range}; +use core::prelude::{Ord, Ordering, PtrExt, Some, range}; use core::ptr; use core::slice as core_slice; use self::Direction::*; diff --git a/src/libcollections/str.rs b/src/libcollections/str.rs index 881eb45f7cc..225814673c4 100644 --- a/src/libcollections/str.rs +++ b/src/libcollections/str.rs @@ -1768,19 +1768,12 @@ impl StrExt for str {} #[cfg(test)] mod tests { - use std::iter::AdditiveIterator; - use std::iter::range; - use std::default::Default; - use std::char::Char; - use std::clone::Clone; - use std::cmp::{Ord, PartialOrd, Equiv}; - use std::cmp::Ordering::{Equal, Greater, Less}; - use std::option::Option::{mod, Some, None}; - use std::result::Result::{Ok, Err}; - use std::ptr::RawPtr; - use std::iter::{Iterator, IteratorExt, DoubleEndedIteratorExt}; + use prelude::*; - use super::*; + use core::default::Default; + use core::iter::AdditiveIterator; + use super::{eq_slice, from_utf8, is_utf8, is_utf16, raw}; + use super::truncate_utf16_at_nul; use super::MaybeOwned::{Owned, Slice}; use std::slice::{AsSlice, SliceExt}; use string::{String, ToString}; diff --git a/src/libcore/prelude.rs b/src/libcore/prelude.rs index f6abc8da79c..fd1598db8cd 100644 --- a/src/libcore/prelude.rs +++ b/src/libcore/prelude.rs @@ -57,7 +57,7 @@ pub use iter::{IteratorOrdExt, MutableDoubleEndedIterator, ExactSizeIterator}; pub use num::{ToPrimitive, FromPrimitive}; pub use option::Option; pub use option::Option::{Some, None}; -pub use ptr::RawPtr; +pub use ptr::{PtrExt, MutPtrExt}; pub use result::Result; pub use result::Result::{Ok, Err}; pub use str::{Str, StrExt}; diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 8c724b4d852..75bb8d33ea8 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -16,11 +16,10 @@ //! typically limited to a few patterns. //! //! Use the [`null` function](fn.null.html) to create null pointers, -//! the [`is_null`](trait.RawPtr.html#tymethod.is_null) -//! and [`is_not_null`](trait.RawPtr.html#method.is_not_null) -//! methods of the [`RawPtr` trait](trait.RawPtr.html) to check for null. -//! The `RawPtr` trait is imported by the prelude, so `is_null` etc. -//! work everywhere. The `RawPtr` also defines the `offset` method, +//! the [`is_null`](trait.PtrExt.html#tymethod.is_null) +//! methods of the [`PtrExt` trait](trait.PtrExt.html) to check for null. +//! The `PtrExt` trait is imported by the prelude, so `is_null` etc. +//! work everywhere. The `PtrExt` also defines the `offset` method, //! for pointer math. //! //! # Common ways to create unsafe pointers @@ -87,16 +86,16 @@ //! but C APIs hand out a lot of pointers generally, so are a common source //! of unsafe pointers in Rust. +#![stable] + use mem; use clone::Clone; use intrinsics; +use option::Option::{mod, Some, None}; use kinds::{Send, Sync}; -use option::Option; -use option::Option::{Some, None}; use cmp::{PartialEq, Eq, Ord, PartialOrd, Equiv}; -use cmp::Ordering; -use cmp::Ordering::{Less, Equal, Greater}; +use cmp::Ordering::{mod, Less, Equal, Greater}; // FIXME #19649: instrinsic docs don't render, so these have no docs :( @@ -121,7 +120,7 @@ pub use intrinsics::set_memory; /// assert!(p.is_null()); /// ``` #[inline] -#[unstable = "may need a different name after pending changes to pointer types"] +#[stable] pub fn null() -> *const T { 0 as *const T } /// Creates a null mutable raw pointer. @@ -135,31 +134,31 @@ pub fn null() -> *const T { 0 as *const T } /// assert!(p.is_null()); /// ``` #[inline] -#[unstable = "may need a different name after pending changes to pointer types"] +#[stable] pub fn null_mut() -> *mut T { 0 as *mut T } -/// Zeroes out `count * size_of::` bytes of memory at `dst`. `count` may be `0`. +/// Zeroes out `count * size_of::` bytes of memory at `dst`. `count` may be +/// `0`. /// /// # Safety /// -/// Beyond accepting a raw pointer, this is unsafe because it will not drop the contents of `dst`, -/// and may be used to create invalid instances of `T`. +/// Beyond accepting a raw pointer, this is unsafe because it will not drop the +/// contents of `dst`, and may be used to create invalid instances of `T`. #[inline] -#[experimental = "uncertain about naming and semantics"] -#[allow(experimental)] +#[unstable = "may play a larger role in std::ptr future extensions"] pub unsafe fn zero_memory(dst: *mut T, count: uint) { set_memory(dst, 0, count); } /// Swaps the values at two mutable locations of the same type, without -/// deinitialising either. They may overlap, unlike `mem::swap` which is otherwise -/// equivalent. +/// deinitialising either. They may overlap, unlike `mem::swap` which is +/// otherwise equivalent. /// /// # Safety /// /// This is only unsafe because it accepts a raw pointer. #[inline] -#[unstable] +#[stable] pub unsafe fn swap(x: *mut T, y: *mut T) { // Give ourselves some scratch space to work with let mut tmp: T = mem::uninitialized(); @@ -183,7 +182,7 @@ pub unsafe fn swap(x: *mut T, y: *mut T) { /// This is only unsafe because it accepts a raw pointer. /// Otherwise, this operation is identical to `mem::replace`. #[inline] -#[unstable] +#[stable] pub unsafe fn replace(dest: *mut T, mut src: T) -> T { mem::swap(mem::transmute(dest), &mut src); // cannot overlap src @@ -201,7 +200,7 @@ pub unsafe fn replace(dest: *mut T, mut src: T) -> T { /// `zero_memory`, or `copy_memory`). Note that `*src = foo` counts as a use /// because it will attempt to drop the value previously at `*src`. #[inline(always)] -#[unstable] +#[stable] pub unsafe fn read(src: *const T) -> T { let mut tmp: T = mem::uninitialized(); copy_nonoverlapping_memory(&mut tmp, src, 1); @@ -214,8 +213,7 @@ pub unsafe fn read(src: *const T) -> T { /// /// This is unsafe for the same reasons that `read` is unsafe. #[inline(always)] -#[experimental] -#[allow(experimental)] +#[unstable = "may play a larger role in std::ptr future extensions"] pub unsafe fn read_and_zero(dest: *mut T) -> T { // Copy the data out from `dest`: let tmp = read(&*dest); @@ -226,8 +224,8 @@ pub unsafe fn read_and_zero(dest: *mut T) -> T { tmp } -/// Overwrites a memory location with the given value without reading or dropping -/// the old value. +/// Overwrites a memory location with the given value without reading or +/// dropping the old value. /// /// # Safety /// @@ -235,36 +233,44 @@ pub unsafe fn read_and_zero(dest: *mut T) -> T { /// not drop the contents of `dst`. This could leak allocations or resources, /// so care must be taken not to overwrite an object that should be dropped. /// -/// This is appropriate for initializing uninitialized memory, or overwritting memory -/// that has previously been `read` from. +/// This is appropriate for initializing uninitialized memory, or overwritting +/// memory that has previously been `read` from. #[inline] -#[unstable] +#[stable] pub unsafe fn write(dst: *mut T, src: T) { intrinsics::move_val_init(&mut *dst, src) } /// Methods on raw pointers -pub trait RawPtr { - /// Returns a null raw pointer. +#[stable] +pub trait PtrExt { + /// Returns the null pointer. + #[deprecated = "call ptr::null instead"] fn null() -> Self; /// Returns true if the pointer is null. - fn is_null(&self) -> bool; + #[stable] + fn is_null(self) -> bool; - /// Returns true if the pointer is not null. - fn is_not_null(&self) -> bool { !self.is_null() } + /// Returns true if the pointer is not equal to the null pointer. + #[deprecated = "use !p.is_null() instead"] + fn is_not_null(self) -> bool { !self.is_null() } - /// Returns the address of the pointer. - fn to_uint(&self) -> uint; + /// Returns true if the pointer is not null. + #[deprecated = "use `as uint` instead"] + fn to_uint(self) -> uint; - /// Returns `None` if the pointer is null, or else returns a reference to the - /// value wrapped in `Some`. + /// Returns `None` if the pointer is null, or else returns a reference to + /// the value wrapped in `Some`. /// /// # Safety /// - /// While this method and its mutable counterpart are useful for null-safety, - /// it is important to note that this is still an unsafe operation because - /// the returned value could be pointing to invalid memory. + /// While this method and its mutable counterpart are useful for + /// null-safety, it is important to note that this is still an unsafe + /// operation because the returned value could be pointing to invalid + /// memory. + #[unstable = "Option is not clearly the right return type, and we may want \ + to tie the return lifetime to a borrow of the raw pointer"] unsafe fn as_ref<'a>(&self) -> Option<&'a T>; /// Calculates the offset from a pointer. `count` is in units of T; e.g. a @@ -272,39 +278,51 @@ pub trait RawPtr { /// /// # Safety /// - /// The offset must be in-bounds of the object, or one-byte-past-the-end. Otherwise - /// `offset` invokes Undefined Behaviour, regardless of whether the pointer is used. + /// The offset must be in-bounds of the object, or one-byte-past-the-end. + /// Otherwise `offset` invokes Undefined Behaviour, regardless of whether + /// the pointer is used. + #[stable] unsafe fn offset(self, count: int) -> Self; } /// Methods on mutable raw pointers -pub trait RawMutPtr{ - /// Returns `None` if the pointer is null, or else returns a mutable reference - /// to the value wrapped in `Some`. +#[stable] +pub trait MutPtrExt{ + /// Returns `None` if the pointer is null, or else returns a mutable + /// reference to the value wrapped in `Some`. /// /// # Safety /// /// As with `as_ref`, this is unsafe because it cannot verify the validity /// of the returned pointer. + #[unstable = "Option is not clearly the right return type, and we may want \ + to tie the return lifetime to a borrow of the raw pointer"] unsafe fn as_mut<'a>(&self) -> Option<&'a mut T>; } -impl RawPtr for *const T { +#[stable] +impl PtrExt for *const T { #[inline] + #[deprecated = "call ptr::null instead"] fn null() -> *const T { null() } #[inline] - fn is_null(&self) -> bool { *self == RawPtr::null() } + #[stable] + fn is_null(self) -> bool { self as uint == 0 } #[inline] - fn to_uint(&self) -> uint { *self as uint } + #[deprecated = "use `as uint` instead"] + fn to_uint(self) -> uint { self as uint } #[inline] + #[stable] unsafe fn offset(self, count: int) -> *const T { intrinsics::offset(self, count) } #[inline] + #[unstable = "return value does not necessarily convey all possible \ + information"] unsafe fn as_ref<'a>(&self) -> Option<&'a T> { if self.is_null() { None @@ -314,22 +332,29 @@ impl RawPtr for *const T { } } -impl RawPtr for *mut T { +#[stable] +impl PtrExt for *mut T { #[inline] + #[deprecated = "call ptr::null instead"] fn null() -> *mut T { null_mut() } #[inline] - fn is_null(&self) -> bool { *self == RawPtr::null() } + #[stable] + fn is_null(self) -> bool { self as uint == 0 } #[inline] - fn to_uint(&self) -> uint { *self as uint } + #[deprecated = "use `as uint` instead"] + fn to_uint(self) -> uint { self as uint } #[inline] + #[stable] unsafe fn offset(self, count: int) -> *mut T { intrinsics::offset(self as *const T, count) as *mut T } #[inline] + #[unstable = "return value does not necessarily convey all possible \ + information"] unsafe fn as_ref<'a>(&self) -> Option<&'a T> { if self.is_null() { None @@ -339,8 +364,11 @@ impl RawPtr for *mut T { } } -impl RawMutPtr for *mut T { +#[stable] +impl MutPtrExt for *mut T { #[inline] + #[unstable = "return value does not necessarily convey all possible \ + information"] unsafe fn as_mut<'a>(&self) -> Option<&'a mut T> { if self.is_null() { None @@ -510,28 +538,33 @@ impl PartialOrd for *mut T { /// raw `*mut T` (which conveys no particular ownership semantics). /// Useful for building abstractions like `Vec` or `Box`, which /// internally use raw pointers to manage the memory that they own. +#[unstable = "recently added to this module"] pub struct Unique(pub *mut T); /// `Unique` pointers are `Send` if `T` is `Send` because the data they /// reference is unaliased. Note that this aliasing invariant is /// unenforced by the type system; the abstraction using the /// `Unique` must enforce it. +#[unstable = "recently added to this module"] unsafe impl Send for Unique { } /// `Unique` pointers are `Sync` if `T` is `Sync` because the data they /// reference is unaliased. Note that this aliasing invariant is /// unenforced by the type system; the abstraction using the /// `Unique` must enforce it. +#[unstable = "recently added to this module"] unsafe impl Sync for Unique { } impl Unique { /// Returns a null Unique. + #[unstable = "recently added to this module"] pub fn null() -> Unique { - Unique(RawPtr::null()) + Unique(null_mut()) } /// Return an (unsafe) pointer into the memory owned by `self`. + #[unstable = "recently added to this module"] pub unsafe fn offset(self, offset: int) -> *mut T { - (self.0 as *const T).offset(offset) as *mut T + self.0.offset(offset) } } diff --git a/src/libcore/slice.rs b/src/libcore/slice.rs index 26684864c4c..87b1961a760 100644 --- a/src/libcore/slice.rs +++ b/src/libcore/slice.rs @@ -47,7 +47,7 @@ use ops::{FnMut, mod}; use option::Option; use option::Option::{None, Some}; use ptr; -use ptr::RawPtr; +use ptr::PtrExt; use mem; use mem::size_of; use kinds::{Sized, marker}; @@ -1334,7 +1334,7 @@ pub unsafe fn from_raw_mut_buf<'a, T>(p: &'a *mut T, len: uint) -> &'a mut [T] { #[deprecated] pub mod raw { use mem::transmute; - use ptr::RawPtr; + use ptr::PtrExt; use raw::Slice; use ops::FnOnce; use option::Option; diff --git a/src/libcore/str/mod.rs b/src/libcore/str/mod.rs index 1e7fe8f060c..34e47c51cfe 100644 --- a/src/libcore/str/mod.rs +++ b/src/libcore/str/mod.rs @@ -28,7 +28,7 @@ use mem; use num::Int; use ops::{Fn, FnMut}; use option::Option::{mod, None, Some}; -use ptr::RawPtr; +use ptr::PtrExt; use raw::{Repr, Slice}; use result::Result::{mod, Ok, Err}; use slice::{mod, SliceExt}; @@ -1072,7 +1072,7 @@ const TAG_CONT_U8: u8 = 0b1000_0000u8; /// Unsafe operations #[deprecated] pub mod raw { - use ptr::RawPtr; + use ptr::PtrExt; use raw::Slice; use slice::SliceExt; use str::StrExt; diff --git a/src/librustc_trans/trans/builder.rs b/src/librustc_trans/trans/builder.rs index 1b9c9d221b9..5249f59d78f 100644 --- a/src/librustc_trans/trans/builder.rs +++ b/src/librustc_trans/trans/builder.rs @@ -501,7 +501,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { debug!("Store {} -> {}", self.ccx.tn().val_to_string(val), self.ccx.tn().val_to_string(ptr)); - assert!(self.llbuilder.is_not_null()); + assert!(!self.llbuilder.is_null()); self.count_insn("store"); unsafe { llvm::LLVMBuildStore(self.llbuilder, val, ptr); @@ -512,7 +512,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { debug!("Store {} -> {}", self.ccx.tn().val_to_string(val), self.ccx.tn().val_to_string(ptr)); - assert!(self.llbuilder.is_not_null()); + assert!(!self.llbuilder.is_null()); self.count_insn("store.volatile"); unsafe { let insn = llvm::LLVMBuildStore(self.llbuilder, val, ptr); diff --git a/src/librustc_trans/trans/value.rs b/src/librustc_trans/trans/value.rs index 4f9b8c5ea37..9e959ce4221 100644 --- a/src/librustc_trans/trans/value.rs +++ b/src/librustc_trans/trans/value.rs @@ -20,7 +20,7 @@ pub struct Value(pub ValueRef); macro_rules! opt_val { ($e:expr) => ( unsafe { match $e { - p if p.is_not_null() => Some(Value(p)), + p if !p.is_null() => Some(Value(p)), _ => None } } @@ -37,7 +37,7 @@ impl Value { pub fn get_parent(self) -> Option { unsafe { match llvm::LLVMGetInstructionParent(self.get()) { - p if p.is_not_null() => Some(BasicBlock(p)), + p if !p.is_null() => Some(BasicBlock(p)), _ => None } } @@ -77,7 +77,7 @@ impl Value { pub fn get_first_use(self) -> Option { unsafe { match llvm::LLVMGetFirstUse(self.get()) { - u if u.is_not_null() => Some(Use(u)), + u if !u.is_null() => Some(Use(u)), _ => None } } @@ -119,7 +119,7 @@ impl Value { /// Tests if this value is a terminator instruction pub fn is_a_terminator_inst(self) -> bool { unsafe { - llvm::LLVMIsATerminatorInst(self.get()).is_not_null() + !llvm::LLVMIsATerminatorInst(self.get()).is_null() } } } @@ -142,7 +142,7 @@ impl Use { pub fn get_next_use(self) -> Option { unsafe { match llvm::LLVMGetNextUse(self.get()) { - u if u.is_not_null() => Some(Use(u)), + u if !u.is_null() => Some(Use(u)), _ => None } } diff --git a/src/libstd/c_vec.rs b/src/libstd/c_vec.rs index f4338815f75..0aa51ee66ed 100644 --- a/src/libstd/c_vec.rs +++ b/src/libstd/c_vec.rs @@ -40,7 +40,7 @@ use mem; use ops::{Drop, FnOnce}; use option::Option; use option::Option::{Some, None}; -use ptr::RawPtr; +use ptr::PtrExt; use ptr; use raw; use slice::AsSlice; diff --git a/src/libstd/collections/hash/table.rs b/src/libstd/collections/hash/table.rs index 3ae3a8ffbad..86c0fc708a5 100644 --- a/src/libstd/collections/hash/table.rs +++ b/src/libstd/collections/hash/table.rs @@ -23,7 +23,7 @@ use num::{Int, UnsignedInt}; use ops::{Deref, DerefMut, Drop}; use option::Option; use option::Option::{Some, None}; -use ptr::{Unique, RawPtr, copy_nonoverlapping_memory, zero_memory}; +use ptr::{Unique, PtrExt, copy_nonoverlapping_memory, zero_memory}; use ptr; use rt::heap::{allocate, deallocate}; diff --git a/src/libstd/io/extensions.rs b/src/libstd/io/extensions.rs index c1f1a5b7869..e8765e3c231 100644 --- a/src/libstd/io/extensions.rs +++ b/src/libstd/io/extensions.rs @@ -22,7 +22,7 @@ use num::Int; use ops::FnOnce; use option::Option; use option::Option::{Some, None}; -use ptr::RawPtr; +use ptr::PtrExt; use result::Result::{Ok, Err}; use slice::{SliceExt, AsSlice}; diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index b6f8bb25b65..ada57bde74c 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -935,7 +935,7 @@ impl<'a> Reader for &'a mut (Reader+'a) { // API yet. If so, it should be a method on Vec. unsafe fn slice_vec_capacity<'a, T>(v: &'a mut Vec, start: uint, end: uint) -> &'a mut [T] { use raw::Slice; - use ptr::RawPtr; + use ptr::PtrExt; assert!(start <= end); assert!(end <= v.capacity()); diff --git a/src/libstd/os.rs b/src/libstd/os.rs index ceb9a4102f6..04f66c2c171 100644 --- a/src/libstd/os.rs +++ b/src/libstd/os.rs @@ -46,7 +46,8 @@ use option::Option; use option::Option::{Some, None}; use path::{Path, GenericPath, BytesContainer}; use sys; -use ptr::RawPtr; +use sys::os as os_imp; +use ptr::PtrExt; use ptr; use result::Result; use result::Result::{Err, Ok}; diff --git a/src/libstd/prelude.rs b/src/libstd/prelude.rs index d1540f98a23..fc59f06ae6c 100644 --- a/src/libstd/prelude.rs +++ b/src/libstd/prelude.rs @@ -73,7 +73,7 @@ #[doc(no_inline)] pub use option::Option; #[doc(no_inline)] pub use option::Option::{Some, None}; #[doc(no_inline)] pub use path::{GenericPath, Path, PosixPath, WindowsPath}; -#[doc(no_inline)] pub use ptr::{RawPtr, RawMutPtr}; +#[doc(no_inline)] pub use ptr::{PtrExt, MutPtrExt}; #[doc(no_inline)] pub use result::Result; #[doc(no_inline)] pub use result::Result::{Ok, Err}; #[doc(no_inline)] pub use io::{Buffer, Writer, Reader, Seek, BufferPrelude}; diff --git a/src/libstd/sys/common/net.rs b/src/libstd/sys/common/net.rs index 382f6875b28..793e81e1ab5 100644 --- a/src/libstd/sys/common/net.rs +++ b/src/libstd/sys/common/net.rs @@ -269,7 +269,7 @@ pub fn get_host_addresses(host: Option<&str>, servname: Option<&str>, // Collect all the results we found let mut addrs = Vec::new(); let mut rp = res; - while rp.is_not_null() { + while !rp.is_null() { unsafe { let addr = try!(sockaddr_to_addr(mem::transmute((*rp).ai_addr), (*rp).ai_addrlen as uint)); diff --git a/src/libstd/sys/unix/backtrace.rs b/src/libstd/sys/unix/backtrace.rs index 983d0e5fa14..ddae9a132c3 100644 --- a/src/libstd/sys/unix/backtrace.rs +++ b/src/libstd/sys/unix/backtrace.rs @@ -244,7 +244,7 @@ fn print(w: &mut Writer, idx: int, addr: *mut libc::c_void) -> IoResult<()> { use iter::{Iterator, IteratorExt}; use os; use path::GenericPath; - use ptr::RawPtr; + use ptr::PtrExt; use ptr; use slice::SliceExt; -- cgit 1.4.1-3-g733a5