Skip to content

Commit 13408f6

Browse files
committed
Fix leak on panic in insert_many.
Fixes #208.
1 parent d85325d commit 13408f6

File tree

2 files changed

+54
-16
lines changed

2 files changed

+54
-16
lines changed

src/lib.rs

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ use core::hint::unreachable_unchecked;
9898
use core::iter::{repeat, FromIterator, FusedIterator, IntoIterator};
9999
use core::mem;
100100
use core::mem::MaybeUninit;
101-
use core::ops::{self, RangeBounds};
101+
use core::ops::{self, Range, RangeBounds};
102102
use core::ptr::{self, NonNull};
103103
use core::slice::{self, SliceIndex};
104104

@@ -975,8 +975,6 @@ impl<A: Array> SmallVec<A> {
975975

976976
/// Insert multiple elements at position `index`, shifting all following elements toward the
977977
/// back.
978-
///
979-
/// Note: when the iterator panics, this can leak memory.
980978
pub fn insert_many<I: IntoIterator<Item = A::Item>>(&mut self, index: usize, iterable: I) {
981979
let iter = iterable.into_iter();
982980
if index == self.len() {
@@ -991,27 +989,41 @@ impl<A: Array> SmallVec<A> {
991989
unsafe {
992990
let old_len = self.len();
993991
assert!(index <= old_len);
994-
let mut ptr = self.as_mut_ptr().add(index);
992+
let start = self.as_mut_ptr();
993+
let mut ptr = start.add(index);
995994

996995
// Move the trailing elements.
997996
ptr::copy(ptr, ptr.add(lower_size_bound), old_len - index);
998997

999998
// In case the iterator panics, don't double-drop the items we just copied above.
1000-
self.set_len(index);
999+
self.set_len(0);
1000+
let mut guard = DropOnPanic {
1001+
start,
1002+
skip: index..(index + lower_size_bound),
1003+
len: old_len + lower_size_bound,
1004+
};
10011005

10021006
let mut num_added = 0;
10031007
for element in iter {
10041008
let mut cur = ptr.add(num_added);
10051009
if num_added >= lower_size_bound {
10061010
// Iterator provided more elements than the hint. Move trailing items again.
10071011
self.reserve(1);
1008-
ptr = self.as_mut_ptr().add(index);
1012+
let start = self.as_mut_ptr();
1013+
ptr = start.add(index);
10091014
cur = ptr.add(num_added);
10101015
ptr::copy(cur, cur.add(1), old_len - index);
1016+
1017+
guard.start = start;
1018+
guard.len += 1;
1019+
guard.skip.end += 1;
10111020
}
10121021
ptr::write(cur, element);
1022+
guard.skip.start += 1;
10131023
num_added += 1;
10141024
}
1025+
mem::forget(guard);
1026+
10151027
if num_added < lower_size_bound {
10161028
// Iterator provided fewer elements than the hint
10171029
ptr::copy(
@@ -1023,6 +1035,24 @@ impl<A: Array> SmallVec<A> {
10231035

10241036
self.set_len(old_len + num_added);
10251037
}
1038+
1039+
struct DropOnPanic<T> {
1040+
start: *mut T,
1041+
skip: Range<usize>,
1042+
len: usize,
1043+
}
1044+
1045+
impl<T> Drop for DropOnPanic<T> {
1046+
fn drop(&mut self) {
1047+
for i in 0..self.len {
1048+
if !self.skip.contains(&i) {
1049+
unsafe {
1050+
ptr::drop_in_place(self.start.add(i));
1051+
}
1052+
}
1053+
}
1054+
}
1055+
}
10261056
}
10271057

10281058
/// Convert a SmallVec to a Vec, without reallocating if the SmallVec has already spilled onto
@@ -1249,6 +1279,22 @@ impl<A: Array> SmallVec<A> {
12491279
data: SmallVecData::from_heap(ptr, length),
12501280
}
12511281
}
1282+
1283+
/// Returns a raw pointer to the vector's buffer.
1284+
pub fn as_ptr(&self) -> *const A::Item {
1285+
// We shadow the slice method of the same name to avoid going through
1286+
// `deref`, which creates an intermediate reference that may place
1287+
// additional safety constraints on the contents of the slice.
1288+
self.triple().0
1289+
}
1290+
1291+
/// Returns a raw mutable pointer to the vector's buffer.
1292+
pub fn as_mut_ptr(&mut self) -> *mut A::Item {
1293+
// We shadow the slice method of the same name to avoid going through
1294+
// `deref_mut`, which creates an intermediate reference that may place
1295+
// additional safety constraints on the contents of the slice.
1296+
self.triple_mut().0
1297+
}
12521298
}
12531299

12541300
impl<A: Array> SmallVec<A>

src/tests.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -345,27 +345,19 @@ fn test_insert_many_panic() {
345345
}
346346
}
347347

348-
// These boxes are leaked on purpose by panicking `insert_many`,
349-
// so we clean them up manually to appease Miri's leak checker.
350-
let mut box1 = Box::new(false);
351-
let mut box2 = Box::new(false);
352-
353348
let mut vec: SmallVec<[PanicOnDoubleDrop; 0]> = vec![
354349
PanicOnDoubleDrop {
355-
dropped: unsafe { Box::from_raw(&mut *box1) },
350+
dropped: Box::new(false),
356351
},
357352
PanicOnDoubleDrop {
358-
dropped: unsafe { Box::from_raw(&mut *box2) },
353+
dropped: Box::new(false),
359354
},
360355
]
361356
.into();
362357
let result = ::std::panic::catch_unwind(move || {
363358
vec.insert_many(0, BadIter);
364359
});
365360
assert!(result.is_err());
366-
367-
drop(box1);
368-
drop(box2);
369361
}
370362

371363
#[test]

0 commit comments

Comments
 (0)