Auto merge of #15 - bluss:drop-nodrop, r=SimonSapin

Fix panic safety issue in drop

The summary is: SmallVec::drop first attempts to drop every element,
then it inhibits the drop of the inner array. The panic safety issue is
that a panic during drop of an element means the inhibition is never
reached, so the inner data can be dropped again.

If Drop is split betweeen SmallVec and SmallVecData, this issue is
avoided because the SmallVecData drop will be called even in the panic
case.

This solution incurs the overhead of an additional drop flag on
SmallVecData.

Fixes #14

Author: bors-servo

lib.rs

@@ -83,6 +83,23 @@ enum SmallVecData<A: Array> {
     Heap { ptr: *mut A::Item, capacity: usize },
 }
 
+impl<A: Array> Drop for SmallVecData<A> {
+    fn drop(&mut self) {
+        unsafe {
+            match *self {
+                ref mut inline @ Inline { .. } => {
+                    // Inhibit the array destructor.
+                    ptr::write(inline, Heap {
+                        ptr: ptr::null_mut(),
+                        capacity: 0,
+                    });
+                }
+                Heap { ptr, capacity } => deallocate(ptr, capacity),
+            }
+        }
+    }
+}
+
 
 pub struct SmallVec<A: Array> {
     len: usize,
@@ -378,22 +395,13 @@ impl<A: Array> SmallVec<A> {
 
 impl<A: Array> Drop for SmallVec<A> {
     fn drop(&mut self) {
+        // Note on panic safety: dropping an element may panic,
+        // but the inner SmallVecData destructor will still run
         unsafe {
             let ptr = self.as_ptr();
             for i in 0 .. self.len {
                 ptr::read(ptr.offset(i as isize));
             }
-
-            match self.data {
-                Inline { .. } => {
-                    // Inhibit the array destructor.
-                    ptr::write(&mut self.data, Heap {
-                        ptr: ptr::null_mut(),
-                        capacity: 0,
-                    });
-                }
-                Heap { ptr, capacity } => deallocate(ptr, capacity),
-            }
         }
     }
 }
@@ -578,4 +586,21 @@ pub mod tests {
 
         assert_eq!(&v.iter().map(|v| **v).collect::<Vec<_>>(), &[0, 3, 2]);
     }
+
+    #[test]
+    #[should_panic]
+    fn test_drop_panic_smallvec() {
+        // This test should only panic once, and not double panic,
+        // which would mean a double drop
+        struct DropPanic;
+
+        impl Drop for DropPanic {
+            fn drop(&mut self) {
+                panic!("drop");
+            }
+        }
+
+        let mut v = SmallVec::<[_; 1]>::new();
+        v.push(DropPanic);
+    }
 }