std: add an RAII unlocker to Mutex.

This automatically unlocks its lock when it goes out of scope, and
provides a safe(ish) method to call .wait.

6 files changed, 105 insertions, 57 deletions

diff --git a/src/libstd/comm/shared.rs b/src/libstd/comm/shared.rs
index 77bf2d7a68d..fcd00b70dd1 100644
--- a/src/libstd/comm/shared.rs
+++ b/src/libstd/comm/shared.rs
@@ -75,7 +75,7 @@ impl<T: Send> Packet<T> {
             select_lock: unsafe { Mutex::new() },
         };
         // see comments in inherit_blocker about why we grab this lock
-        unsafe { p.select_lock.lock() }
+        unsafe { p.select_lock.lock_noguard() }
         return p;
     }
 
@@ -124,7 +124,7 @@ impl<T: Send> Packet<T> {
         // interfere with this method. After we unlock this lock, we're
         // signifying that we're done modifying self.cnt and self.to_wake and
         // the port is ready for the world to continue using it.
-        unsafe { self.select_lock.unlock() }
+        unsafe { self.select_lock.unlock_noguard() }
     }
 
     pub fn send(&mut self, t: T) -> bool {
@@ -438,8 +438,7 @@ impl<T: Send> Packet<T> {
         // about looking at and dealing with to_wake. Once we have acquired the
         // lock, we are guaranteed that inherit_blocker is done.
         unsafe {
-            self.select_lock.lock();
-            self.select_lock.unlock();
+            let _guard = self.select_lock.lock();
         }
 
         // Like the stream implementation, we want to make sure that the count
diff --git a/src/libstd/os.rs b/src/libstd/os.rs
index 20d1ae2f3e2..4a5958c2cfb 100644
--- a/src/libstd/os.rs
+++ b/src/libstd/os.rs
@@ -44,7 +44,6 @@ use ptr;
 use str;
 use str::{Str, StrSlice};
 use fmt;
-use unstable::finally::Finally;
 use sync::atomics::{AtomicInt, INIT_ATOMIC_INT, SeqCst};
 use path::{Path, GenericPath};
 use iter::Iterator;
@@ -146,15 +145,12 @@ Serialize access through a global lock.
 */
 fn with_env_lock<T>(f: || -> T) -> T {
     use unstable::mutex::{Mutex, MUTEX_INIT};
-    use unstable::finally::Finally;
 
     static mut lock: Mutex = MUTEX_INIT;
 
     unsafe {
-        return (|| {
-            lock.lock();
-            f()
-        }).finally(|| lock.unlock());
+        let _guard = lock.lock();
+        f()
     }
 }
 
diff --git a/src/libstd/rt/args.rs b/src/libstd/rt/args.rs
index c417ea375fd..c91797c9559 100644
--- a/src/libstd/rt/args.rs
+++ b/src/libstd/rt/args.rs
@@ -68,7 +68,6 @@ mod imp {
     use option::{Option, Some, None};
     use ptr::RawPtr;
     use iter::Iterator;
-    use unstable::finally::Finally;
     use unstable::mutex::{Mutex, MUTEX_INIT};
     use mem;
 
@@ -111,16 +110,10 @@ mod imp {
     }
 
     fn with_lock<T>(f: || -> T) -> T {
-        (|| {
-            unsafe {
-                lock.lock();
-                f()
-            }
-        }).finally(|| {
-            unsafe {
-                lock.unlock();
-            }
-        })
+        unsafe {
+            let _guard = lock.lock();
+            f()
+        }
     }
 
     fn get_global_ptr() -> *mut Option<~~[~[u8]]> {
diff --git a/src/libstd/unstable/dynamic_lib.rs b/src/libstd/unstable/dynamic_lib.rs
index 8529b69c6eb..4828c4ee5af 100644
--- a/src/libstd/unstable/dynamic_lib.rs
+++ b/src/libstd/unstable/dynamic_lib.rs
@@ -157,7 +157,7 @@ pub mod dl {
         unsafe {
             // dlerror isn't thread safe, so we need to lock around this entire
             // sequence
-            lock.lock();
+            let _guard = lock.lock();
             let _old_error = dlerror();
 
             let result = f();
@@ -168,7 +168,7 @@ pub mod dl {
             } else {
                 Err(str::raw::from_c_str(last_error))
             };
-            lock.unlock();
+
             ret
         }
     }
diff --git a/src/libstd/unstable/mutex.rs b/src/libstd/unstable/mutex.rs
index 3122e925e82..2fa7fbeab4e 100644
--- a/src/libstd/unstable/mutex.rs
+++ b/src/libstd/unstable/mutex.rs
@@ -47,10 +47,24 @@
 
 #[allow(non_camel_case_types)];
 
+use option::{Option, None, Some};
+use ops::Drop;
+
 pub struct Mutex {
     priv inner: imp::Mutex,
 }
 
+/// Automatically unlocks the mutex that it was created from on
+/// destruction.
+///
+/// Using this makes lock-based code resilient to unwinding/task
+/// failure, because the lock will be automatically unlocked even
+/// then.
+#[must_use]
+pub struct LockGuard<'a> {
+    priv lock: &'a mut Mutex
+}
+
 pub static MUTEX_INIT: Mutex = Mutex {
     inner: imp::MUTEX_INIT,
 };
@@ -63,23 +77,62 @@ impl Mutex {
 
     /// Acquires this lock. This assumes that the current thread does not
     /// already hold the lock.
-    pub unsafe fn lock(&mut self) { self.inner.lock() }
+    ///
+    /// # Example
+    /// ```
+    /// use std::unstable::mutex::Mutex;
+    /// unsafe {
+    ///     let mut lock = Mutex::new();
+    ///
+    ///     {
+    ///         let _guard = lock.lock();
+    ///         // critical section...
+    ///     } // automatically unlocked in `_guard`'s destructor
+    /// }
+    /// ```
+    pub unsafe fn lock<'a>(&'a mut self) -> LockGuard<'a> {
+        self.inner.lock();
+
+        LockGuard { lock: self }
+    }
 
-    /// Attempts to acquire the lock. The value returned is whether the lock was
-    /// acquired or not
-    pub unsafe fn trylock(&mut self) -> bool { self.inner.trylock() }
+    /// Attempts to acquire the lock. The value returned is `Some` if
+    /// the attempt succeeded.
+    pub unsafe fn trylock<'a>(&'a mut self) -> Option<LockGuard<'a>> {
+        if self.inner.trylock() {
+            Some(LockGuard { lock: self })
+        } else {
+            None
+        }
+    }
+
+    /// Acquire the lock without creating a `LockGuard`.
+    ///
+    /// Prefer using `.lock`.
+    pub unsafe fn lock_noguard(&mut self) { self.inner.lock() }
+
+    /// Attempts to acquire the lock without creating a
+    /// `LockGuard`. The value returned is whether the lock was
+    /// acquired or not.
+    ///
+    /// Prefer using `.trylock`.
+    pub unsafe fn trylock_noguard(&mut self) -> bool {
+        self.inner.trylock()
+    }
 
     /// Unlocks the lock. This assumes that the current thread already holds the
     /// lock.
-    pub unsafe fn unlock(&mut self) { self.inner.unlock() }
+    pub unsafe fn unlock_noguard(&mut self) { self.inner.unlock() }
 
     /// Block on the internal condition variable.
     ///
-    /// This function assumes that the lock is already held
-    pub unsafe fn wait(&mut self) { self.inner.wait() }
+    /// This function assumes that the lock is already held. Prefer
+    /// using `LockGuard.wait` since that guarantees that the lock is
+    /// held.
+    pub unsafe fn wait_noguard(&mut self) { self.inner.wait() }
 
     /// Signals a thread in `wait` to wake up
-    pub unsafe fn signal(&mut self) { self.inner.signal() }
+    pub unsafe fn signal_noguard(&mut self) { self.inner.signal() }
 
     /// This function is especially unsafe because there are no guarantees made
     /// that no other thread is currently holding the lock or waiting on the
@@ -87,6 +140,25 @@ impl Mutex {
     pub unsafe fn destroy(&mut self) { self.inner.destroy() }
 }
 
+impl<'a> LockGuard<'a> {
+    /// Block on the internal condition variable.
+    pub unsafe fn wait(&mut self) {
+        self.lock.wait_noguard()
+    }
+
+    /// Signals a thread in `wait` to wake up.
+    pub unsafe fn signal(&mut self) {
+        self.lock.signal_noguard()
+    }
+}
+
+#[unsafe_destructor]
+impl<'a> Drop for LockGuard<'a> {
+    fn drop(&mut self) {
+        unsafe {self.lock.unlock_noguard()}
+    }
+}
+
 #[cfg(unix)]
 mod imp {
     use libc;
@@ -382,6 +454,7 @@ mod imp {
 mod test {
     use prelude::*;
 
+    use mem::drop;
     use super::{Mutex, MUTEX_INIT};
     use rt::thread::Thread;
 
@@ -389,8 +462,7 @@ mod test {
     fn somke_lock() {
         static mut lock: Mutex = MUTEX_INIT;
         unsafe {
-            lock.lock();
-            lock.unlock();
+            let _guard = lock.lock();
         }
     }
 
@@ -398,14 +470,14 @@ mod test {
     fn somke_cond() {
         static mut lock: Mutex = MUTEX_INIT;
         unsafe {
-            lock.lock();
+            let mut guard = lock.lock();
             let t = Thread::start(proc() {
-                lock.lock();
-                lock.signal();
-                lock.unlock();
+                let mut guard = lock.lock();
+                guard.signal();
             });
-            lock.wait();
-            lock.unlock();
+            guard.wait();
+            drop(guard);
+
             t.join();
         }
     }
diff --git a/src/libstd/unstable/sync.rs b/src/libstd/unstable/sync.rs
index c2fa168a478..343eacbf429 100644
--- a/src/libstd/unstable/sync.rs
+++ b/src/libstd/unstable/sync.rs
@@ -11,16 +11,16 @@
 use clone::Clone;
 use kinds::Send;
 use ops::Drop;
-use option::{Option,Some,None};
+use option::Option;
 use sync::arc::UnsafeArc;
-use unstable::mutex::Mutex;
+use unstable::mutex::{Mutex, LockGuard};
 
 pub struct LittleLock {
     priv l: Mutex,
 }
 
 pub struct LittleGuard<'a> {
-    priv l: &'a mut Mutex,
+    priv l: LockGuard<'a>
 }
 
 impl Drop for LittleLock {
@@ -29,33 +29,21 @@ impl Drop for LittleLock {
     }
 }
 
-#[unsafe_destructor]
-impl<'a> Drop for LittleGuard<'a> {
-    fn drop(&mut self) {
-        unsafe { self.l.unlock(); }
-    }
-}
-
 impl LittleLock {
     pub fn new() -> LittleLock {
         unsafe { LittleLock { l: Mutex::new() } }
     }
 
     pub unsafe fn lock<'a>(&'a mut self) -> LittleGuard<'a> {
-        self.l.lock();
-        LittleGuard { l: &mut self.l }
+        LittleGuard { l: self.l.lock() }
     }
 
     pub unsafe fn try_lock<'a>(&'a mut self) -> Option<LittleGuard<'a>> {
-        if self.l.trylock() {
-            Some(LittleGuard { l: &mut self.l })
-        } else {
-            None
-        }
+        self.l.trylock().map(|guard| LittleGuard { l: guard })
     }
 
     pub unsafe fn signal(&mut self) {
-        self.l.signal();
+        self.l.signal_noguard();
     }
 }