libstd: Change atomically to use RAII.

Author: pcwalton

src/libstd/rt/comm.rs

@@ -565,7 +565,7 @@ impl<'self, T: Send> SelectPortInner<T> for &'self Port<T> {
 impl<'self, T: Send> SelectPort<T> for &'self Port<T> { }
 
 pub struct SharedChan<T> {
-    // Just like Chan, but a shared AtomicOption instead of Cell
+    // Just like Chan, but a shared AtomicOption
     priv next: UnsafeArc<AtomicOption<StreamChanOne<T>>>
 }
 
@@ -716,7 +716,6 @@ mod test {
     use super::*;
     use option::*;
     use rt::test::*;
-    use cell::Cell;
     use num::Times;
     use rt::util;
 
@@ -1113,7 +1112,7 @@ mod test {
 
     #[test]
     fn send_deferred() {
-        use unstable::sync::atomically;
+        use unstable::sync::atomic;
 
         // Tests no-rescheduling of send_deferred on all types of channels.
         do run_in_newsched_task {
@@ -1129,15 +1128,12 @@ mod test {
             let p_mp = mp.clone();
             do spawntask { p_mp.recv(); }
 
-            let cs = Cell::new((cone, cstream, cshared, mp));
             unsafe {
-                atomically(|| {
-                    let (cone, cstream, cshared, mp) = cs.take();
-                    cone.send_deferred(());
-                    cstream.send_deferred(());
-                    cshared.send_deferred(());
-                    mp.send_deferred(());
-                })
+                let _guard = atomic();
+                cone.send_deferred(());
+                cstream.send_deferred(());
+                cshared.send_deferred(());
+                mp.send_deferred(());
             }
         }
     }

src/libstd/unstable/dynamic_lib.rs

@@ -140,7 +140,7 @@ pub mod dl {
     use path;
     use ptr;
     use str;
-    use unstable::sync::atomically;
+    use unstable::sync::atomic;
     use result::*;
 
     pub unsafe fn open_external(filename: &path::Path) -> *libc::c_void {
@@ -158,25 +158,24 @@ pub mod dl {
         static mut lock: Mutex = MUTEX_INIT;
         unsafe {
             // dlerror isn't thread safe, so we need to lock around this entire
-            // sequence. `atomically` asserts that we don't do anything that
+            // sequence. `atomic` asserts that we don't do anything that
             // would cause this task to be descheduled, which could deadlock
             // the scheduler if it happens while the lock is held.
             // FIXME #9105 use a Rust mutex instead of C++ mutexes.
-            atomically(|| {
-                lock.lock();
-                let _old_error = dlerror();
-
-                let result = f();
-
-                let last_error = dlerror();
-                let ret = if ptr::null() == last_error {
-                    Ok(result)
-                } else {
-                    Err(str::raw::from_c_str(last_error))
-                };
-                lock.unlock();
-                ret
-            })
+            let _guard = atomic();
+            lock.lock();
+            let _old_error = dlerror();
+
+            let result = f();
+
+            let last_error = dlerror();
+            let ret = if ptr::null() == last_error {
+                Ok(result)
+            } else {
+                Err(str::raw::from_c_str(last_error))
+            };
+            lock.unlock();
+            ret
         }
     }
 
@@ -209,7 +208,7 @@ pub mod dl {
     use libc;
     use path;
     use ptr;
-    use unstable::sync::atomically;
+    use unstable::sync::atomic;
     use result::*;
 
     pub unsafe fn open_external(filename: &path::Path) -> *libc::c_void {
@@ -226,18 +225,17 @@ pub mod dl {
 
     pub fn check_for_errors_in<T>(f: || -> T) -> Result<T, ~str> {
         unsafe {
-            atomically(|| {
-                SetLastError(0);
+            let _guard = atomic();
+            SetLastError(0);
 
-                let result = f();
+            let result = f();
 
-                let error = os::errno();
-                if 0 == error {
-                    Ok(result)
-                } else {
-                    Err(format!("Error code {}", error))
-                }
-            })
+            let error = os::errno();
+            if 0 == error {
+                Ok(result)
+            } else {
+                Err(format!("Error code {}", error))
+            }
         }
     }
 

src/libstd/unstable/sync.rs

@@ -14,7 +14,6 @@ use ptr;
 use option::{Option,Some,None};
 use task;
 use unstable::atomics::{AtomicOption,AtomicUint,Acquire,Release,Relaxed,SeqCst};
-use unstable::finally::Finally;
 use unstable::mutex::Mutex;
 use ops::Drop;
 use clone::Clone;
@@ -295,17 +294,44 @@ impl<T> Drop for UnsafeArc<T>{
 
 /****************************************************************************/
 
+pub struct AtomicGuard {
+    on: bool,
+}
+
+impl Drop for AtomicGuard {
+    fn drop(&mut self) {
+        use rt::task::{Task, GreenTask, SchedTask};
+        use rt::local::Local;
+
+        if self.on {
+            unsafe {
+                let task_opt: Option<*mut Task> = Local::try_unsafe_borrow();
+                match task_opt {
+                    Some(t) => {
+                        match (*t).task_type {
+                            GreenTask(_) => (*t).death.allow_deschedule(),
+                            SchedTask => {}
+                        }
+                    }
+                    None => {}
+                }
+            }
+        }
+    }
+}
+
 /**
- * Enables a runtime assertion that no operation in the argument closure shall
- * use scheduler operations (deschedule, recv, spawn, etc). This is for use with
- * pthread mutexes, which may block the entire scheduler thread, rather than
- * just one task, and is hence prone to deadlocks if mixed with descheduling.
+ * Enables a runtime assertion that no operation while the returned guard is
+ * live uses scheduler operations (deschedule, recv, spawn, etc). This is for
+ * use with pthread mutexes, which may block the entire scheduler thread,
+ * rather than just one task, and is hence prone to deadlocks if mixed with
+ * descheduling.
  *
  * NOTE: THIS DOES NOT PROVIDE LOCKING, or any sort of critical-section
  * synchronization whatsoever. It only makes sense to use for CPU-local issues.
  */
 // FIXME(#8140) should not be pub
-pub unsafe fn atomically<U>(f: || -> U) -> U {
+pub unsafe fn atomic() -> AtomicGuard {
     use rt::task::{Task, GreenTask, SchedTask};
     use rt::local::Local;
 
@@ -314,15 +340,19 @@ pub unsafe fn atomically<U>(f: || -> U) -> U {
         Some(t) => {
             match (*t).task_type {
                 GreenTask(_) => {
-                    (|| {
-                        (*t).death.inhibit_deschedule();
-                        f()
-                    }).finally(|| (*t).death.allow_deschedule())
+                    (*t).death.inhibit_deschedule();
+                    return AtomicGuard {
+                        on: true,
+                    };
                 }
-                SchedTask => f()
+                SchedTask => {}
             }
         }
-        None => f()
+        None => {}
+    }
+
+    AtomicGuard {
+        on: false,
     }
 }
 
@@ -481,7 +511,7 @@ mod tests {
     use comm;
     use option::*;
     use prelude::*;
-    use super::{Exclusive, UnsafeArc, atomically};
+    use super::{Exclusive, UnsafeArc, atomic};
     use task;
     use mem::size_of;
 
@@ -493,10 +523,10 @@ mod tests {
     }
 
     #[test]
-    fn test_atomically() {
+    fn test_atomic() {
         // NB. The whole runtime will abort on an 'atomic-sleep' violation,
         // so we can't really test for the converse behaviour.
-        unsafe { atomically(|| ()) } task::deschedule(); // oughtn't fail
+        unsafe { let _ = atomic(); } // oughtn't fail
     }
 
     #[test]