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
This commit is contained in:
bors-servo
2015-08-21 08:11:06 -06:00
+36 -11
View File
@@ -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);
}
}