Merge pull request #73 from FenrirWolf/update-2018-08-18

Update for nightly-2018-08-18

1 files changed, 21 insertions, 15 deletions

diff --git a/ctr-std/src/sys_common/at_exit_imp.rs b/ctr-std/src/sys_common/at_exit_imp.rs
index 26da51c..76e5df2 100644
--- a/ctr-std/src/sys_common/at_exit_imp.rs
+++ b/ctr-std/src/sys_common/at_exit_imp.rs
@@ -14,17 +14,22 @@
 
 use boxed::FnBox;
 use ptr;
+use mem;
 use sys_common::mutex::Mutex;
 
-type Queue = Vec<Box<FnBox()>>;
+type Queue = Vec<Box<dyn FnBox()>>;
 
 // NB these are specifically not types from `std::sync` as they currently rely
 // on poisoning and this module needs to operate at a lower level than requiring
 // the thread infrastructure to be in place (useful on the borders of
 // initialization/destruction).
+// We never call `LOCK.init()`, so it is UB to attempt to
+// acquire this mutex reentrantly!
 static LOCK: Mutex = Mutex::new();
 static mut QUEUE: *mut Queue = ptr::null_mut();
 
+const DONE: *mut Queue = 1_usize as *mut _;
+
 // The maximum number of times the cleanup routines will be run. While running
 // the at_exit closures new ones may be registered, and this count is the number
 // of times the new closures will be allowed to register successfully. After
@@ -35,7 +40,7 @@ unsafe fn init() -> bool {
     if QUEUE.is_null() {
         let state: Box<Queue> = box Vec::new();
         QUEUE = Box::into_raw(state);
-    } else if QUEUE as usize == 1 {
+    } else if QUEUE == DONE {
         // can't re-init after a cleanup
         return false
     }
@@ -44,20 +49,21 @@ unsafe fn init() -> bool {
 }
 
 pub fn cleanup() {
-    for i in 0..ITERS {
+    for i in 1..=ITERS {
         unsafe {
-            LOCK.lock();
-            let queue = QUEUE;
-            QUEUE = if i == ITERS - 1 {1} else {0} as *mut _;
-            LOCK.unlock();
+            let queue = {
+                let _guard = LOCK.lock();
+                mem::replace(&mut QUEUE, if i == ITERS { DONE } else { ptr::null_mut() })
+            };
 
             // make sure we're not recursively cleaning up
-            assert!(queue as usize != 1);
+            assert!(queue != DONE);
 
             // If we never called init, not need to cleanup!
-            if queue as usize != 0 {
+            if !queue.is_null() {
                 let queue: Box<Queue> = Box::from_raw(queue);
                 for to_run in *queue {
+                    // We are not holding any lock, so reentrancy is fine.
                     to_run();
                 }
             }
@@ -65,16 +71,16 @@ pub fn cleanup() {
     }
 }
 
-pub fn push(f: Box<FnBox()>) -> bool {
-    let mut ret = true;
+pub fn push(f: Box<dyn FnBox()>) -> bool {
     unsafe {
-        LOCK.lock();
+        let _guard = LOCK.lock();
         if init() {
+            // We are just moving `f` around, not calling it.
+            // There is no possibility of reentrancy here.
             (*QUEUE).push(f);
+            true
         } else {
-            ret = false;
+            false
         }
-        LOCK.unlock();
     }
-    ret
 }