From 71d4e77db8ad4b6d821da7e5d5300134ac95974e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 24 Nov 2014 11:16:40 -0800 Subject: std: Rewrite the `sync` module This commit is a reimplementation of `std::sync` to be based on the system-provided primitives wherever possible. The previous implementation was fundamentally built on top of channels, and as part of the runtime reform it has become clear that this is not the level of abstraction that the standard level should be providing. This rewrite aims to provide as thin of a shim as possible on top of the system primitives in order to make them safe. The overall interface of the `std::sync` module has in general not changed, but there are a few important distinctions, highlighted below: * The condition variable type, `Condvar`, has been separated out of a `Mutex`. A condition variable is now an entirely separate type. This separation benefits users who only use one mutex, and provides a clearer distinction of who's responsible for managing condition variables (the application). * All of `Condvar`, `Mutex`, and `RWLock` are now directly built on top of system primitives rather than using a custom implementation. The `Once`, `Barrier`, and `Semaphore` types are still built upon these abstractions of the system primitives. * The `Condvar`, `Mutex`, and `RWLock` types all have a new static type and constant initializer corresponding to them. These are provided primarily for C FFI interoperation, but are often useful to otherwise simply have a global lock. The types, however, will leak memory unless `destroy()` is called on them, which is clearly documented. * The `Condvar` implementation for an `RWLock` write lock has been removed. This may be added back in the future with a userspace implementation, but this commit is focused on exposing the system primitives first. * The fundamental architecture of this design is to provide two separate layers. The first layer is that exposed by `sys_common` which is a cross-platform bare-metal abstraction of the system synchronization primitives. No attempt is made at making this layer safe, and it is quite unsafe to use! It is currently not exported as part of the API of the standard library, but the stabilization of the `sys` module will ensure that these will be exposed in time. The purpose of this layer is to provide the core cross-platform abstractions if necessary to implementors. The second layer is the layer provided by `std::sync` which is intended to be the thinnest possible layer on top of `sys_common` which is entirely safe to use. There are a few concerns which need to be addressed when making these system primitives safe: * Once used, the OS primitives can never be **moved**. This means that they essentially need to have a stable address. The static primitives use `&'static self` to enforce this, and the non-static primitives all use a `Box` to provide this guarantee. * Poisoning is leveraged to ensure that invalid data is not accessible from other tasks after one has panicked. In addition to these overall blanket safety limitations, each primitive has a few restrictions of its own: * Mutexes and rwlocks can only be unlocked from the same thread that they were locked by. This is achieved through RAII lock guards which cannot be sent across threads. * Mutexes and rwlocks can only be unlocked if they were previously locked. This is achieved by not exposing an unlocking method. * A condition variable can only be waited on with a locked mutex. This is achieved by requiring a `MutexGuard` in the `wait()` method. * A condition variable cannot be used concurrently with more than one mutex. This is guaranteed by dynamically binding a condition variable to precisely one mutex for its entire lifecycle. This restriction may be able to be relaxed in the future (a mutex is unbound when no threads are waiting on the condvar), but for now it is sufficient to guarantee safety. * Condvars now support timeouts for their blocking operations. The implementation for these operations is provided by the system. Due to the modification of the `Condvar` API, removal of the `std::sync::mutex` API, and reimplementation, this is a breaking change. Most code should be fairly easy to port using the examples in the documentation of these primitives. [breaking-change] Closes #17094 Closes #18003 --- src/libstd/sync/mutex.rs | 364 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 289 insertions(+), 75 deletions(-) (limited to 'src/libstd/sync/mutex.rs') diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index c9e90210c30..3d17f2bc64b 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -8,43 +8,68 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -//! A simple native mutex implementation. Warning: this API is likely -//! to change soon. +use prelude::*; -#![allow(dead_code)] - -use core::prelude::*; -use alloc::boxed::Box; -use rustrt::mutex; - -pub const LOCKED: uint = 1 << 0; -pub const BLOCKED: uint = 1 << 1; +use cell::UnsafeCell; +use kinds::marker; +use sync::{poison, AsMutexGuard}; +use sys_common::mutex as sys; /// A mutual exclusion primitive useful for protecting shared data /// -/// This mutex will properly block tasks waiting for the lock to become -/// available. The mutex can also be statically initialized or created via a -/// `new` constructor. +/// This mutex will block threads waiting for the lock to become available. The +/// mutex can also be statically initialized or created via a `new` +/// constructor. Each mutex has a type parameter which represents the data that +/// it is protecting. The data can only be accessed through the RAII guards +/// returned from `lock` and `try_lock`, which guarantees that the data is only +/// ever accessed when the mutex is locked. +/// +/// # 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. /// /// # Example /// -/// ```rust,ignore -/// use std::sync::mutex::Mutex; +/// ```rust +/// use std::sync::{Arc, Mutex}; +/// const N: uint = 10; /// -/// let m = Mutex::new(); -/// let guard = m.lock(); -/// // do some work -/// drop(guard); // unlock the lock +/// // Spawn a few threads to increment a shared variable (non-atomically), and +/// // let the main thread know once all increments are done. +/// // +/// // Here we're using an Arc to share memory among tasks, and the data inside +/// // the Arc is protected with a mutex. +/// let data = Arc::new(Mutex::new(0)); +/// +/// let (tx, rx) = channel(); +/// for _ in range(0, 10) { +/// let (data, tx) = (data.clone(), tx.clone()); +/// spawn(proc() { +/// // 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(); +/// *data += 1; +/// if *data == N { +/// tx.send(()); +/// } +/// // the lock is unlocked here when `data` goes out of scope. +/// }); +/// } +/// +/// rx.recv(); /// ``` -pub struct Mutex { +pub struct Mutex { // Note that this static mutex is in a *box*, not inlined into the struct - // itself. This is done for memory safety reasons with the usage of a - // StaticNativeMutex inside the static mutex above. Once a native mutex has - // been used once, its address can never change (it can't be moved). This - // mutex type can be safely moved at any time, so to ensure that the native - // mutex is used correctly we box the inner lock to give it a constant - // address. - lock: Box, + // itself. Once a native mutex has been used once, its address can never + // change (it can't be moved). This mutex type can be safely moved at any + // time, so to ensure that the native mutex is used correctly we box the + // inner lock to give it a constant address. + inner: Box, + data: UnsafeCell, } /// The static mutex type is provided to allow for static allocation of mutexes. @@ -57,8 +82,8 @@ pub struct Mutex { /// /// # Example /// -/// ```rust,ignore -/// use std::sync::mutex::{StaticMutex, MUTEX_INIT}; +/// ```rust +/// use std::sync::{StaticMutex, MUTEX_INIT}; /// /// static LOCK: StaticMutex = MUTEX_INIT; /// @@ -69,35 +94,113 @@ pub struct Mutex { /// // lock is unlocked here. /// ``` pub struct StaticMutex { - lock: mutex::StaticNativeMutex, + lock: sys::Mutex, + poison: UnsafeCell, } /// An RAII implementation of a "scoped lock" of a mutex. When this structure is /// dropped (falls out of scope), the lock will be unlocked. +/// +/// The data protected by the mutex can be access through this guard via its +/// Deref and DerefMut implementations #[must_use] -pub struct Guard<'a> { - guard: mutex::LockGuard<'a>, +pub struct MutexGuard<'a, T: 'a> { + // funny underscores due to how Deref/DerefMut currently work (they + // disregard field privacy). + __lock: &'a Mutex, + __guard: StaticMutexGuard, } -fn lift_guard(guard: mutex::LockGuard) -> Guard { - Guard { guard: guard } +/// 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>, } /// Static initialization of a mutex. This constant can be used to initialize /// other mutex constants. pub const MUTEX_INIT: StaticMutex = StaticMutex { - lock: mutex::NATIVE_MUTEX_INIT + lock: sys::MUTEX_INIT, + poison: UnsafeCell { value: poison::Flag { failed: false } }, }; -impl StaticMutex { - /// Attempts to grab this lock, see `Mutex::try_lock` - pub fn try_lock<'a>(&'a self) -> Option> { - unsafe { self.lock.trylock().map(lift_guard) } +impl Mutex { + /// Creates a new mutex in an unlocked state ready for use. + pub fn new(t: T) -> Mutex { + Mutex { + inner: box MUTEX_INIT, + data: UnsafeCell::new(t), + } + } + + /// Acquires a mutex, blocking the current task until it is able to do so. + /// + /// This function will block the local task until it is available to acquire + /// the mutex. Upon returning, the task is the only task with the 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 + /// + /// 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()) + } + } + + /// Attempts to acquire this lock. + /// + /// If the lock could not be acquired at this time, then `None` is returned. + /// Otherwise, an RAII guard is returned. The lock will be unlocked when the + /// guard is dropped. + /// + /// This function does not block. + /// + /// # Panics + /// + /// If another user of this mutex panicked while holding the mutex, then + /// this call will immediately panic 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) + }) + } } +} +#[unsafe_destructor] +impl Drop for Mutex { + fn drop(&mut self) { + // This is actually safe b/c we know that there is no further usage of + // this mutex (it's up to the user to arrange for a mutex to get + // dropped, that's not our job) + unsafe { self.inner.lock.destroy() } + } +} + +impl StaticMutex { /// Acquires this lock, see `Mutex::lock` - pub fn lock<'a>(&'a self) -> Guard<'a> { - lift_guard(unsafe { self.lock.lock() }) + pub fn lock(&'static self) -> StaticMutexGuard { + unsafe { self.lock.lock() } + StaticMutexGuard::new(self) + } + + /// Attempts to grab this lock, see `Mutex::try_lock` + pub fn try_lock(&'static self) -> Option { + if unsafe { self.lock.try_lock() } { + Some(StaticMutexGuard::new(self)) + } else { + None + } } /// Deallocates resources associated with this static mutex. @@ -110,58 +213,73 @@ impl StaticMutex { /// *all* platforms. It may be the case that some platforms do not leak /// memory if this method is not called, but this is not guaranteed to be /// true on all platforms. - pub unsafe fn destroy(&self) { + pub unsafe fn destroy(&'static self) { self.lock.destroy() } } -impl Mutex { - /// Creates a new mutex in an unlocked state ready for use. - pub fn new() -> Mutex { - Mutex { - lock: box StaticMutex { - lock: unsafe { mutex::StaticNativeMutex::new() }, - } - } +impl<'mutex, T> MutexGuard<'mutex, T> { + fn new(lock: &Mutex, guard: StaticMutexGuard) -> MutexGuard { + MutexGuard { __lock: lock, __guard: guard } } +} - /// Attempts to acquire this lock. - /// - /// If the lock could not be acquired at this time, then `None` is returned. - /// Otherwise, an RAII guard is returned. The lock will be unlocked when the - /// guard is dropped. - /// - /// This function does not block. - pub fn try_lock<'a>(&'a self) -> Option> { - self.lock.try_lock() +impl<'mutex, T> AsMutexGuard for MutexGuard<'mutex, T> { + unsafe fn as_mutex_guard(&self) -> &StaticMutexGuard { &self.__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() } } +} - /// Acquires a mutex, blocking the current task until it is able to do so. - /// - /// This function will block the local task until it is available to acquire - /// the mutex. Upon returning, the task is the only task with the 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. - pub fn lock<'a>(&'a self) -> Guard<'a> { self.lock.lock() } +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; + } + } +} + +pub fn guard_lock(guard: &StaticMutexGuard) -> &sys::Mutex { guard.lock } +pub fn guard_poison(guard: &StaticMutexGuard) -> &poison::Guard { + &guard.poison +} + +impl AsMutexGuard for StaticMutexGuard { + unsafe fn as_mutex_guard(&self) -> &StaticMutexGuard { self } } -impl Drop for Mutex { +#[unsafe_destructor] +impl Drop for StaticMutexGuard { fn drop(&mut self) { - // This is actually safe b/c we know that there is no further usage of - // this mutex (it's up to the user to arrange for a mutex to get - // dropped, that's not our job) - unsafe { self.lock.destroy() } + unsafe { + self.poison.done(); + self.lock.unlock(); + } } } #[cfg(test)] mod test { use prelude::*; - use super::{Mutex, StaticMutex, MUTEX_INIT}; + + use task; + use sync::{Arc, Mutex, StaticMutex, MUTEX_INIT, Condvar}; #[test] fn smoke() { - let m = Mutex::new(); + let m = Mutex::new(()); drop(m.lock()); drop(m.lock()); } @@ -211,8 +329,104 @@ mod test { } #[test] - fn trylock() { - let m = Mutex::new(); + fn try_lock() { + let m = Mutex::new(()); assert!(m.try_lock().is_some()); } + + #[test] + fn test_mutex_arc_condvar() { + let arc = Arc::new((Mutex::new(false), Condvar::new())); + let arc2 = arc.clone(); + let (tx, rx) = channel(); + spawn(proc() { + // wait until parent gets in + rx.recv(); + let &(ref lock, ref cvar) = &*arc2; + let mut lock = lock.lock(); + *lock = true; + cvar.notify_one(); + }); + + let &(ref lock, ref cvar) = &*arc; + let lock = lock.lock(); + tx.send(()); + assert!(!*lock); + while !*lock { + cvar.wait(&lock); + } + } + + #[test] + #[should_fail] + fn test_arc_condvar_poison() { + let arc = Arc::new((Mutex::new(1i), Condvar::new())); + let arc2 = arc.clone(); + let (tx, rx) = channel(); + + spawn(proc() { + rx.recv(); + let &(ref lock, ref cvar) = &*arc2; + let _g = lock.lock(); + cvar.notify_one(); + // Parent should fail when it wakes up. + panic!(); + }); + + let &(ref lock, ref cvar) = &*arc; + let lock = lock.lock(); + tx.send(()); + while *lock == 1 { + cvar.wait(&lock); + } + } + + #[test] + #[should_fail] + fn test_mutex_arc_poison() { + let arc = Arc::new(Mutex::new(1i)); + let arc2 = arc.clone(); + let _ = task::try(proc() { + let lock = arc2.lock(); + assert_eq!(*lock, 2); + }); + let lock = arc.lock(); + assert_eq!(*lock, 1); + } + + #[test] + fn test_mutex_arc_nested() { + // Tests nested mutexes and access + // to underlying data. + let arc = Arc::new(Mutex::new(1i)); + let arc2 = Arc::new(Mutex::new(arc)); + let (tx, rx) = channel(); + spawn(proc() { + let lock = arc2.lock(); + let lock2 = lock.deref().lock(); + assert_eq!(*lock2, 1); + tx.send(()); + }); + rx.recv(); + } + + #[test] + fn test_mutex_arc_access_in_unwind() { + let arc = Arc::new(Mutex::new(1i)); + let arc2 = arc.clone(); + let _ = task::try::<()>(proc() { + struct Unwinder { + i: Arc>, + } + impl Drop for Unwinder { + fn drop(&mut self) { + *self.i.lock() += 1; + } + } + let _u = Unwinder { i: arc2 }; + panic!(); + }); + let lock = arc.lock(); + assert_eq!(*lock, 2); + } } -- cgit 1.4.1-3-g733a5