Skip to content

Commit

Permalink
Fix panic safety issue in drop by using NoDrop
Browse files Browse the repository at this point in the history
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.

NoDrop acts as a panic guard; its drop will trigger on panic, so the
inner array's drop will be inhibited even on panic during SmallVec::drop.

Using NoDrop incurs the overhead of its enum tag and its drop flag.

Fixes servo#14
  • Loading branch information
bluss committed Aug 20, 2015
1 parent bff26fc commit dc1de27
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 10 deletions.
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ documentation = "http://doc.servo.org/smallvec/"
name = "smallvec"
path = "lib.rs"
doctest = false

[dependencies]
nodrop = "0.1.4"
16 changes: 6 additions & 10 deletions lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

//! Small vectors in various sizes. These store a certain number of elements inline and fall back
//! to the heap for larger allocations.
extern crate nodrop;

use std::cmp;
use std::fmt;
Expand All @@ -13,8 +14,9 @@ use std::ops;
use std::ptr;
use std::slice;

use SmallVecData::{Inline, Heap};
use nodrop::NoDrop;

use SmallVecData::{Inline, Heap};

pub trait VecLike<T>:
ops::Index<usize, Output=T> +
Expand Down Expand Up @@ -79,7 +81,7 @@ impl<'a, T: 'a> Drop for SmallVecMoveIterator<'a,T> {
}

enum SmallVecData<A: Array> {
Inline { array: A },
Inline { array: NoDrop<A> },
Heap { ptr: *mut A::Item, capacity: usize },
}

Expand Down Expand Up @@ -370,7 +372,7 @@ impl<A: Array> SmallVec<A> {
unsafe {
SmallVec {
len: 0,
data: Inline { array: mem::zeroed() },
data: Inline { array: NoDrop::new(mem::zeroed()) },
}
}
}
Expand All @@ -385,13 +387,7 @@ impl<A: Array> Drop for SmallVec<A> {
}

match self.data {
Inline { .. } => {
// Inhibit the array destructor.
ptr::write(&mut self.data, Heap {
ptr: ptr::null_mut(),
capacity: 0,
});
}
Inline { .. } => { /* NoDrop inhibits drop */ }
Heap { ptr, capacity } => deallocate(ptr, capacity),
}
}
Expand Down

0 comments on commit dc1de27

Please sign in to comment.