Add a safe implementation of MutexArc::access* methods

Current access methods are nestable and unsafe. This patch renames
current methods implementation - prepends unsafe_ - and implements 2 new
methods that are both safe and un-nestable.

Fixes #7473

Author: flaper87

src/libextra/arc.rs

@@ -159,7 +159,9 @@ impl<T:Freeze + Send> Clone for Arc<T> {
 
 #[doc(hidden)]
 struct MutexArcInner<T> { priv lock: Mutex, priv failed: bool, priv data: T }
+
 /// An Arc with mutable data protected by a blocking mutex.
+#[no_freeze]
 struct MutexArc<T> { priv x: UnsafeArc<MutexArcInner<T>> }
 
 
@@ -190,6 +192,35 @@ impl<T:Send> MutexArc<T> {
         MutexArc { x: UnsafeArc::new(data) }
     }
 
+
+    /// Refer unsafe_access and access methods for the documentaiton. 
+    #[inline]
+    unsafe fn lock_and_access<U>(&self, blk: &fn(x: &mut T) -> U) -> U {
+        let state = self.x.get();
+        // Borrowck would complain about this if the function were
+        // not already unsafe. See borrow_rwlock, far below.
+        do (&(*state).lock).lock {
+            check_poison(true, (*state).failed);
+            let _z = PoisonOnFail(&mut (*state).failed);
+            blk(&mut (*state).data)
+        }
+    }
+
+    #[inline]
+    unsafe fn lock_and_access_cond<'x, 'c, U>(&self,
+                                         blk: &fn(x: &'x mut T,
+                                                  c: &'c Condvar) -> U)
+                                         -> U {
+        let state = self.x.get();
+        do (&(*state).lock).lock_cond |cond| {
+            check_poison(true, (*state).failed);
+            let _z = PoisonOnFail(&mut (*state).failed);
+            blk(&mut (*state).data,
+                &Condvar {is_mutex: true,
+                          failed: &mut (*state).failed,
+                          cond: cond })
+        }
+    }
     /**
      * Access the underlying mutable data with mutual exclusion from other
      * tasks. The argument closure will be run with the mutex locked; all
@@ -215,31 +246,16 @@ impl<T:Send> MutexArc<T> {
      */
     #[inline]
     pub unsafe fn unsafe_access<U>(&self, blk: &fn(x: &mut T) -> U) -> U {
-        let state = self.x.get();
-        // Borrowck would complain about this if the function were
-        // not already unsafe. See borrow_rwlock, far below.
-        do (&(*state).lock).lock {
-            check_poison(true, (*state).failed);
-            let _z = PoisonOnFail(&mut (*state).failed);
-            blk(&mut (*state).data)
-        }
+        self.lock_and_access(blk)
     }
 
-    /// As access(), but with a condvar, as sync::mutex.lock_cond().
+    /// As unsafe_access(), but with a condvar, as sync::mutex.lock_cond().
     #[inline]
     pub unsafe fn unsafe_access_cond<'x, 'c, U>(&self,
                                          blk: &fn(x: &'x mut T,
                                                   c: &'c Condvar) -> U)
                                          -> U {
-        let state = self.x.get();
-        do (&(*state).lock).lock_cond |cond| {
-            check_poison(true, (*state).failed);
-            let _z = PoisonOnFail(&mut (*state).failed);
-            blk(&mut (*state).data,
-                &Condvar {is_mutex: true,
-                          failed: &mut (*state).failed,
-                          cond: cond })
-        }
+        self.lock_and_access_cond(blk)
     }
 
     /**
@@ -259,6 +275,34 @@ impl<T:Send> MutexArc<T> {
     }
 }
 
+impl<T:Freeze + Send> MutexArc<T> {
+
+    /**
+     * As unsafe_access.
+     *
+     * The difference between access and unsafe_access is that the former
+     * forbids mutexes to be nested. The purpose of this is to offer a safe
+     * implementation of both methods access and access_cond to be used instead
+     * of rwlock in cases where no readers are needed and sightly better performance
+     * is required.
+     *
+     * Both methods have the same failure behaviour as unsafe_access and
+     * unsafe_access_cond.
+     */
+    #[inline]
+    pub fn access<U>(&self, blk: &fn(x: &mut T) -> U) -> U {
+        unsafe { self.lock_and_access(blk) }
+    }
+    
+    #[inline]
+    pub fn access_cond<'x, 'c, U>(&self,
+                                  blk: &fn(x: &'x mut T,
+                                           c: &'c Condvar) -> U)
+                                  -> U {
+        unsafe { self.lock_and_access_cond(blk) }
+    }
+}
+
 // Common code for {mutex.access,rwlock.write}{,_cond}.
 #[inline]
 #[doc(hidden)]
@@ -589,6 +633,100 @@ mod tests {
 
     #[test]
     fn test_mutex_arc_condvar() {
+        let arc = ~MutexArc::new(false);
+        let arc2 = ~arc.clone();
+        let (p,c) = comm::oneshot();
+        let (c,p) = (Cell::new(c), Cell::new(p));
+        do task::spawn || {
+            // wait until parent gets in
+            p.take().recv();
+            do arc2.access_cond |state, cond| {
+                *state = true;
+                cond.signal();
+            }
+        }
+
+        do arc.access_cond |state, cond| {
+            c.take().send(());
+            assert!(!*state);
+            while !*state {
+                cond.wait();
+            }
+        }
+    }
+
+    #[test] #[should_fail]
+    fn test_arc_condvar_poison() {
+        let arc = ~MutexArc::new(1);
+        let arc2 = ~arc.clone();
+        let (p, c) = comm::stream();
+
+        do task::spawn_unlinked || {
+            let _ = p.recv();
+            do arc2.access_cond |one, cond| {
+                cond.signal();
+                // Parent should fail when it wakes up.
+                assert_eq!(*one, 0);
+            }
+        }
+
+        do arc.access_cond |one, cond| {
+            c.send(());
+            while *one == 1 {
+                cond.wait();
+            }
+        }
+    }
+
+    #[test] #[should_fail]
+    fn test_mutex_arc_poison() {
+        let arc = ~MutexArc::new(1);
+        let arc2 = ~arc.clone();
+        do task::try || {
+            do arc2.access |one| {
+                assert_eq!(*one, 2);
+            }
+        };
+        do arc.access |one| {
+            assert_eq!(*one, 1);
+        }
+    }
+
+    #[test] #[should_fail]
+    pub fn test_mutex_arc_unwrap_poison() {
+        let arc = MutexArc::new(1);
+        let arc2 = ~(&arc).clone();
+        let (p, c) = comm::stream();
+        do task::spawn {
+            do arc2.access |one| {
+                c.send(());
+                assert!(*one == 2);
+            }
+        }
+        let _ = p.recv();
+        let one = arc.unwrap();
+        assert!(one == 1);
+    }
+    
+    #[test]
+    fn test_unsafe_mutex_arc_nested() {
+        unsafe {
+            // Tests nested mutexes and access
+            // to underlaying data.
+            let arc = ~MutexArc::new(1);
+            let arc2 = ~MutexArc::new(*arc);
+            do task::spawn || {
+                do (*arc2).unsafe_access |mutex| {
+                    do (*mutex).access |one| {
+                        assert!(*one == 1);
+                    }
+                }
+            };
+        } 
+    }
+
+    #[test]
+    fn test_unsafe_mutex_arc_condvar() {
         unsafe {
             let arc = MutexArc::new(false);
             let arc2 = arc.clone();
@@ -613,7 +751,7 @@ mod tests {
     }
 
     #[test] #[should_fail]
-    fn test_arc_condvar_poison() {
+    fn test_unsafe_arc_condvar_poison() {
         unsafe {
             let arc = MutexArc::new(1);
             let arc2 = arc.clone();
@@ -637,7 +775,7 @@ mod tests {
         }
     }
     #[test] #[should_fail]
-    fn test_mutex_arc_poison() {
+    fn test_unsafe_mutex_arc_poison() {
         unsafe {
             let arc = MutexArc::new(1);
             let arc2 = arc.clone();
@@ -651,8 +789,9 @@ mod tests {
             }
         }
     }
+
     #[test] #[should_fail]
-    pub fn test_mutex_arc_unwrap_poison() {
+    pub fn test_unsafe_mutex_arc_unwrap_poison() {
         let arc = MutexArc::new(1);
         let arc2 = arc.clone();
         let (p, c) = comm::stream();
@@ -668,6 +807,7 @@ mod tests {
         let one = arc.unwrap();
         assert!(one == 1);
     }
+
     #[test] #[should_fail]
     fn test_rw_arc_poison_wr() {
         let arc = RWArc::new(1);
@@ -681,6 +821,7 @@ mod tests {
             assert_eq!(*one, 1);
         }
     }
+
     #[test] #[should_fail]
     fn test_rw_arc_poison_ww() {
         let arc = RWArc::new(1);

src/test/compile-fail/mutex-arc-nested.rs

@@ -0,0 +1,26 @@
+// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+extern mod extra;
+
+use std::task;
+use extra::arc::{MutexArc};
+
+fn test_mutex_arc_nested() {
+    let arc = ~MutexArc::new(1);
+    let arc2 = ~MutexArc::new(*arc);
+
+    do task::spawn || {
+        do (*arc2).access |mutex| { // This should fail because MutexArc is not Freeze
+        }
+    };
+}
+
+fn main() {}