fix(vm): Handle out of order drops with RootedValue

vm/src/api/opaque.rs

@@ -3,17 +3,21 @@ use std::cmp::Ordering;
 use std::fmt;
 use std::marker::PhantomData;
 use std::ops::Deref;
-use std::result::Result as StdResult;
 
 use base::types::ArcType;
 
 use api::{ArrayRef, Getable, Pushable, ValueRef, VmType};
-use thread::{ActiveThread, RootedThread, RootedValue, Thread, ThreadInternal, VmRoot};
+use thread::{ActiveThread, RootedValue, Thread, ThreadInternal, VmRoot};
 use types::{VmIndex, VmInt};
 use value::{ArrayRepr, Value, ValueArray};
 use vm;
 use {Result, Variants};
 
+#[cfg(feature = "serde")]
+use std::result::Result as StdResult;
+#[cfg(feature = "serde")]
+use thread::RootedThread;
+
 #[cfg(feature = "serde")]
 use serde::de::{Deserialize, Deserializer};
 #[cfg(feature = "serde")]

vm/src/gc.rs

@@ -19,11 +19,12 @@ use {Error, Result};
 #[inline]
 unsafe fn allocate(size: usize) -> *mut u8 {
     // Allocate an extra element if it does not fit exactly
-    let cap = size / mem::size_of::<f64>() + (if size % mem::size_of::<f64>() != 0 {
-        1
-    } else {
-        0
-    });
+    let cap = size / mem::size_of::<f64>()
+        + (if size % mem::size_of::<f64>() != 0 {
+            1
+        } else {
+            0
+        });
     ptr_from_vec(Vec::<f64>::with_capacity(cap))
 }
 
@@ -37,11 +38,12 @@ fn ptr_from_vec(mut buf: Vec<f64>) -> *mut u8 {
 
 #[inline]
 unsafe fn deallocate(ptr: *mut u8, old_size: usize) {
-    let cap = old_size / mem::size_of::<f64>() + (if old_size % mem::size_of::<f64>() != 0 {
-        1
-    } else {
-        0
-    });
+    let cap = old_size / mem::size_of::<f64>()
+        + (if old_size % mem::size_of::<f64>() != 0 {
+            1
+        } else {
+            0
+        });
     Vec::<f64>::from_raw_parts(ptr as *mut f64, 0, cap);
 }
 
@@ -425,6 +427,10 @@ impl<T: ?Sized> GcPtr<T> {
         unsafe { &(*self.header().type_info).fields_key }
     }
 
+    pub fn ptr_eq(self, other: GcPtr<T>) -> bool {
+        ptr::eq(&*self, &*other)
+    }
+
     fn header(&self) -> &GcHeader {
         // Use of transmute_copy allows us to get the pointer
         // to the data regardless of wether T is unsized or not
@@ -1001,8 +1007,8 @@ mod tests {
         let dropped = Rc::new(Cell::new(false));
         let mut gc = Gc::new(Generation::default(), usize::MAX);
         {
-            let ptr =
-                gc.alloc(Move(Dropable {
+            let ptr = gc
+                .alloc(Move(Dropable {
                     dropped: dropped.clone(),
                 })).unwrap();
             assert_eq!(false, ptr.dropped.get());

vm/src/thread.rs

@@ -130,7 +130,6 @@ pub enum Status {
 }
 
 /// A rooted value
-#[derive(Clone)]
 pub struct RootedValue<T>
 where
     T: Deref<Target = Thread>,
@@ -139,6 +138,23 @@ where
     value: Value,
 }
 
+impl<T> Clone for RootedValue<T>
+where
+    T: Deref<Target = Thread> + Clone,
+{
+    fn clone(&self) -> Self {
+        self.vm
+            .rooted_values
+            .write()
+            .unwrap()
+            .push(self.value.clone());
+        RootedValue {
+            vm: self.vm.clone(),
+            value: self.value.clone(),
+        }
+    }
+}
+
 impl<T, U> PartialEq<RootedValue<U>> for RootedValue<T>
 where
     T: Deref<Target = Thread>,
@@ -154,8 +170,12 @@ where
     T: Deref<Target = Thread>,
 {
     fn drop(&mut self) {
-        // TODO not safe if the root changes order of being dropped with another root
-        self.vm.rooted_values.write().unwrap().pop();
+        let mut rooted_values = self.vm.rooted_values.write().unwrap();
+        let i = rooted_values
+            .iter()
+            .position(|p| p.obj_eq(&self.value))
+            .unwrap_or_else(|| ice!("Rooted value has already been dropped"));
+        rooted_values.swap_remove(i);
     }
 }
 
@@ -176,6 +196,7 @@ where
     where
         U: VmRoot<'vm>,
     {
+        vm.rooted_values.write().unwrap().push(self.value.clone());
         RootedValue {
             vm,
             value: self.value.clone(),