Skip to content

Commit 27899e3

Browse files
authored
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
2 parents 69c78a9 + c9595fa commit 27899e3

File tree

2 files changed

+28
-25
lines changed

2 files changed

+28
-25
lines changed

library/alloc/src/vec/mod.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -1619,6 +1619,8 @@ impl<T, A: Allocator> Vec<T, A> {
16191619
let prev_ptr = ptr.add(gap.write.wrapping_sub(1));
16201620

16211621
if same_bucket(&mut *read_ptr, &mut *prev_ptr) {
1622+
// Increase `gap.read` now since the drop may panic.
1623+
gap.read += 1;
16221624
/* We have found duplicate, drop it in-place */
16231625
ptr::drop_in_place(read_ptr);
16241626
} else {
@@ -1631,9 +1633,8 @@ impl<T, A: Allocator> Vec<T, A> {
16311633

16321634
/* We have filled that place, so go further */
16331635
gap.write += 1;
1636+
gap.read += 1;
16341637
}
1635-
1636-
gap.read += 1;
16371638
}
16381639

16391640
/* Technically we could let `gap` clean up with its Drop, but

library/alloc/tests/vec.rs

+25-23
Original file line numberDiff line numberDiff line change
@@ -2234,48 +2234,50 @@ fn test_vec_dedup() {
22342234
#[test]
22352235
fn test_vec_dedup_panicking() {
22362236
#[derive(Debug)]
2237-
struct Panic {
2238-
drop_counter: &'static AtomicU32,
2237+
struct Panic<'a> {
2238+
drop_counter: &'a Cell<u32>,
22392239
value: bool,
22402240
index: usize,
22412241
}
22422242

2243-
impl PartialEq for Panic {
2243+
impl<'a> PartialEq for Panic<'a> {
22442244
fn eq(&self, other: &Self) -> bool {
22452245
self.value == other.value
22462246
}
22472247
}
22482248

2249-
impl Drop for Panic {
2249+
impl<'a> Drop for Panic<'a> {
22502250
fn drop(&mut self) {
2251-
let x = self.drop_counter.fetch_add(1, Ordering::SeqCst);
2252-
assert!(x != 4);
2251+
self.drop_counter.set(self.drop_counter.get() + 1);
2252+
if !std::thread::panicking() {
2253+
assert!(self.index != 4);
2254+
}
22532255
}
22542256
}
22552257

2256-
static DROP_COUNTER: AtomicU32 = AtomicU32::new(0);
2258+
let drop_counter = &Cell::new(0);
22572259
let expected = [
2258-
Panic { drop_counter: &DROP_COUNTER, value: false, index: 0 },
2259-
Panic { drop_counter: &DROP_COUNTER, value: false, index: 5 },
2260-
Panic { drop_counter: &DROP_COUNTER, value: true, index: 6 },
2261-
Panic { drop_counter: &DROP_COUNTER, value: true, index: 7 },
2260+
Panic { drop_counter, value: false, index: 0 },
2261+
Panic { drop_counter, value: false, index: 5 },
2262+
Panic { drop_counter, value: true, index: 6 },
2263+
Panic { drop_counter, value: true, index: 7 },
22622264
];
22632265
let mut vec = vec![
2264-
Panic { drop_counter: &DROP_COUNTER, value: false, index: 0 },
2266+
Panic { drop_counter, value: false, index: 0 },
22652267
// these elements get deduplicated
2266-
Panic { drop_counter: &DROP_COUNTER, value: false, index: 1 },
2267-
Panic { drop_counter: &DROP_COUNTER, value: false, index: 2 },
2268-
Panic { drop_counter: &DROP_COUNTER, value: false, index: 3 },
2269-
Panic { drop_counter: &DROP_COUNTER, value: false, index: 4 },
2270-
// here it panics
2271-
Panic { drop_counter: &DROP_COUNTER, value: false, index: 5 },
2272-
Panic { drop_counter: &DROP_COUNTER, value: true, index: 6 },
2273-
Panic { drop_counter: &DROP_COUNTER, value: true, index: 7 },
2268+
Panic { drop_counter, value: false, index: 1 },
2269+
Panic { drop_counter, value: false, index: 2 },
2270+
Panic { drop_counter, value: false, index: 3 },
2271+
Panic { drop_counter, value: false, index: 4 },
2272+
// here it panics while dropping the item with index==4
2273+
Panic { drop_counter, value: false, index: 5 },
2274+
Panic { drop_counter, value: true, index: 6 },
2275+
Panic { drop_counter, value: true, index: 7 },
22742276
];
22752277

2276-
let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
2277-
vec.dedup();
2278-
}));
2278+
let _ = catch_unwind(AssertUnwindSafe(|| vec.dedup())).unwrap_err();
2279+
2280+
assert_eq!(drop_counter.get(), 4);
22792281

22802282
let ok = vec.iter().zip(expected.iter()).all(|(x, y)| x.index == y.index);
22812283

0 commit comments

Comments
 (0)