auto merge of #8966 : FlaPer87/rust/issue/7473, r=bblum

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: bors

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>> }
 
 
@@ -199,10 +201,10 @@ impl<T:Send> MutexArc<T> {
      * The reason this function is 'unsafe' is because it is possible to
      * construct a circular reference among multiple Arcs by mutating the
      * underlying data. This creates potential for deadlock, but worse, this
-     * will guarantee a memory leak of all involved Arcs. Using mutex Arcs
+     * will guarantee a memory leak of all involved Arcs. Using MutexArcs
      * inside of other Arcs is safe in absence of circular references.
      *
-     * If you wish to nest mutex_arcs, one strategy for ensuring safety at
+     * If you wish to nest MutexArcs, one strategy for ensuring safety at
      * runtime is to add a "nesting level counter" inside the stored data, and
      * when traversing the arcs, assert that they monotonically decrease.
      *
@@ -214,7 +216,7 @@ impl<T:Send> MutexArc<T> {
      * blocked on the mutex) will also fail immediately.
      */
     #[inline]
-    pub unsafe fn access<U>(&self, blk: &fn(x: &mut T) -> U) -> U {
+    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.
@@ -225,9 +227,9 @@ impl<T:Send> MutexArc<T> {
         }
     }
 
-    /// 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 access_cond<'x, 'c, U>(&self,
+    pub unsafe fn unsafe_access_cond<'x, 'c, U>(&self,
                                          blk: &fn(x: &'x mut T,
                                                   c: &'c Condvar) -> U)
                                          -> U {
@@ -259,6 +261,39 @@ 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. While unsafe_access can be used on
+     * MutexArcs without freezable interiors, this safe version of access
+     * requires the Freeze bound, which prohibits access on MutexArcs which
+     * might contain nested MutexArcs inside.
+     *
+     * The purpose of this is to offer a safe implementation of MutexArc to be
+     * used instead of RWArc 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.unsafe_access(blk) }
+    }
+
+    /// As unsafe_access_cond but safe and Freeze.
+    #[inline]
+    pub fn access_cond<'x, 'c, U>(&self,
+                                  blk: &fn(x: &'x mut T,
+                                           c: &'c Condvar) -> U)
+                                  -> U {
+        unsafe { self.unsafe_access_cond(blk) }
+    }
+}
+
 // Common code for {mutex.access,rwlock.write}{,_cond}.
 #[inline]
 #[doc(hidden)]
@@ -589,85 +624,98 @@ mod tests {
 
     #[test]
     fn test_mutex_arc_condvar() {
-        unsafe {
-            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();
-                }
+        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();
-                }
+        }
+
+        do arc.access_cond |state, cond| {
+            c.take().send(());
+            assert!(!*state);
+            while !*state {
+                cond.wait();
             }
         }
     }
 
     #[test] #[should_fail]
     fn test_arc_condvar_poison() {
-        unsafe {
-            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);
-                }
+        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();
-                }
+        do arc.access_cond |one, cond| {
+            c.send(());
+            while *one == 1 {
+                cond.wait();
             }
         }
     }
+
     #[test] #[should_fail]
     fn test_mutex_arc_poison() {
-        unsafe {
-            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);
+        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 arc2 = ~(&arc).clone();
         let (p, c) = comm::stream();
         do task::spawn {
-            unsafe {
-                do arc2.access |one| {
-                    c.send(());
-                    assert!(*one == 2);
-                }
+            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] #[should_fail]
     fn test_rw_arc_poison_wr() {
         let arc = RWArc::new(1);
@@ -681,6 +729,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| { //~ ERROR instantiating a type parameter with an incompatible type
+        }
+    };
+}
+
+fn main() {}