Rollup merge of #85625 - SkiFire13:fix-85613-vec-dedup-drop-panics, r=nagisa

Prevent double drop in `Vec::dedup_by` if a destructor panics

Fixes #85613
This commit is contained in:
Dylan DPC 2021-05-26 13:32:06 +02:00 committed by GitHub
commit 27899e3887
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 28 additions and 25 deletions

View File

@ -1619,6 +1619,8 @@ impl<T, A: Allocator> Vec<T, A> {
let prev_ptr = ptr.add(gap.write.wrapping_sub(1));
if same_bucket(&mut *read_ptr, &mut *prev_ptr) {
// Increase `gap.read` now since the drop may panic.
gap.read += 1;
/* We have found duplicate, drop it in-place */
ptr::drop_in_place(read_ptr);
} else {
@ -1631,9 +1633,8 @@ impl<T, A: Allocator> Vec<T, A> {
/* We have filled that place, so go further */
gap.write += 1;
gap.read += 1;
}
gap.read += 1;
}
/* Technically we could let `gap` clean up with its Drop, but

View File

@ -2234,48 +2234,50 @@ fn test_vec_dedup() {
#[test]
fn test_vec_dedup_panicking() {
#[derive(Debug)]
struct Panic {
drop_counter: &'static AtomicU32,
struct Panic<'a> {
drop_counter: &'a Cell<u32>,
value: bool,
index: usize,
}
impl PartialEq for Panic {
impl<'a> PartialEq for Panic<'a> {
fn eq(&self, other: &Self) -> bool {
self.value == other.value
}
}
impl Drop for Panic {
impl<'a> Drop for Panic<'a> {
fn drop(&mut self) {
let x = self.drop_counter.fetch_add(1, Ordering::SeqCst);
assert!(x != 4);
self.drop_counter.set(self.drop_counter.get() + 1);
if !std::thread::panicking() {
assert!(self.index != 4);
}
}
}
static DROP_COUNTER: AtomicU32 = AtomicU32::new(0);
let drop_counter = &Cell::new(0);
let expected = [
Panic { drop_counter: &DROP_COUNTER, value: false, index: 0 },
Panic { drop_counter: &DROP_COUNTER, value: false, index: 5 },
Panic { drop_counter: &DROP_COUNTER, value: true, index: 6 },
Panic { drop_counter: &DROP_COUNTER, value: true, index: 7 },
Panic { drop_counter, value: false, index: 0 },
Panic { drop_counter, value: false, index: 5 },
Panic { drop_counter, value: true, index: 6 },
Panic { drop_counter, value: true, index: 7 },
];
let mut vec = vec![
Panic { drop_counter: &DROP_COUNTER, value: false, index: 0 },
Panic { drop_counter, value: false, index: 0 },
// these elements get deduplicated
Panic { drop_counter: &DROP_COUNTER, value: false, index: 1 },
Panic { drop_counter: &DROP_COUNTER, value: false, index: 2 },
Panic { drop_counter: &DROP_COUNTER, value: false, index: 3 },
Panic { drop_counter: &DROP_COUNTER, value: false, index: 4 },
// here it panics
Panic { drop_counter: &DROP_COUNTER, value: false, index: 5 },
Panic { drop_counter: &DROP_COUNTER, value: true, index: 6 },
Panic { drop_counter: &DROP_COUNTER, value: true, index: 7 },
Panic { drop_counter, value: false, index: 1 },
Panic { drop_counter, value: false, index: 2 },
Panic { drop_counter, value: false, index: 3 },
Panic { drop_counter, value: false, index: 4 },
// here it panics while dropping the item with index==4
Panic { drop_counter, value: false, index: 5 },
Panic { drop_counter, value: true, index: 6 },
Panic { drop_counter, value: true, index: 7 },
];
let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
vec.dedup();
}));
let _ = catch_unwind(AssertUnwindSafe(|| vec.dedup())).unwrap_err();
assert_eq!(drop_counter.get(), 4);
let ok = vec.iter().zip(expected.iter()).all(|(x, y)| x.index == y.index);