diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index ccc4f03a1e505..aa6c0d1f06dd0 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1370,22 +1370,75 @@ impl Vec { where F: FnMut(&T) -> bool, { - let len = self.len(); - let mut del = 0; - { - let v = &mut **self; - - for i in 0..len { - if !f(&v[i]) { - del += 1; - } else if del > 0 { - v.swap(i - del, i); + let original_len = self.len(); + // Avoid double drop if the drop guard is not executed, + // since we may make some holes during the process. + unsafe { self.set_len(0) }; + + // Vec: [Kept, Kept, Hole, Hole, Hole, Hole, Unchecked, Unchecked] + // |<- processed len ->| ^- next to check + // |<- deleted cnt ->| + // |<- original_len ->| + // Kept: Elements which predicate returns true on. + // Hole: Moved or dropped element slot. + // Unchecked: Unchecked valid elements. + // + // This drop guard will be invoked when predicate or `drop` of element panicked. + // It shifts unchecked elements to cover holes and `set_len` to the correct length. + // In cases when predicate and `drop` never panick, it will be optimized out. + struct BackshiftOnDrop<'a, T, A: Allocator> { + v: &'a mut Vec, + processed_len: usize, + deleted_cnt: usize, + original_len: usize, + } + + impl Drop for BackshiftOnDrop<'_, T, A> { + fn drop(&mut self) { + if self.deleted_cnt > 0 { + // SAFETY: Trailing unchecked items must be valid since we never touch them. + unsafe { + ptr::copy( + self.v.as_ptr().add(self.processed_len), + self.v.as_mut_ptr().add(self.processed_len - self.deleted_cnt), + self.original_len - self.processed_len, + ); + } + } + // SAFETY: After filling holes, all items are in contiguous memory. + unsafe { + self.v.set_len(self.original_len - self.deleted_cnt); } } } - if del > 0 { - self.truncate(len - del); + + let mut g = BackshiftOnDrop { v: self, processed_len: 0, deleted_cnt: 0, original_len }; + + while g.processed_len < original_len { + // SAFETY: Unchecked element must be valid. + let cur = unsafe { &mut *g.v.as_mut_ptr().add(g.processed_len) }; + if !f(cur) { + // Advance early to avoid double drop if `drop_in_place` panicked. + g.processed_len += 1; + g.deleted_cnt += 1; + // SAFETY: We never touch this element again after dropped. + unsafe { ptr::drop_in_place(cur) }; + // We already advanced the counter. + continue; + } + if g.deleted_cnt > 0 { + // SAFETY: `deleted_cnt` > 0, so the hole slot must not overlap with current element. + // We use copy for move, and never touch this element again. + unsafe { + let hole_slot = g.v.as_mut_ptr().add(g.processed_len - g.deleted_cnt); + ptr::copy_nonoverlapping(cur, hole_slot, 1); + } + } + g.processed_len += 1; } + + // All item are processed. This can be optimized to `set_len` by LLVM. + drop(g); } /// Removes all but the first of consecutive elements in the vector that resolve to the same diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index e19406d7a0697..2e56e06b4d0b1 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -287,6 +287,67 @@ fn test_retain() { assert_eq!(vec, [2, 4]); } +#[test] +fn test_retain_pred_panic_with_hole() { + let v = (0..5).map(Rc::new).collect::>(); + catch_unwind(AssertUnwindSafe(|| { + let mut v = v.clone(); + v.retain(|r| match **r { + 0 => true, + 1 => false, + 2 => true, + _ => panic!(), + }); + })) + .unwrap_err(); + // Everything is dropped when predicate panicked. + assert!(v.iter().all(|r| Rc::strong_count(r) == 1)); +} + +#[test] +fn test_retain_pred_panic_no_hole() { + let v = (0..5).map(Rc::new).collect::>(); + catch_unwind(AssertUnwindSafe(|| { + let mut v = v.clone(); + v.retain(|r| match **r { + 0 | 1 | 2 => true, + _ => panic!(), + }); + })) + .unwrap_err(); + // Everything is dropped when predicate panicked. + assert!(v.iter().all(|r| Rc::strong_count(r) == 1)); +} + +#[test] +fn test_retain_drop_panic() { + struct Wrap(Rc); + + impl Drop for Wrap { + fn drop(&mut self) { + if *self.0 == 3 { + panic!(); + } + } + } + + let v = (0..5).map(|x| Rc::new(x)).collect::>(); + catch_unwind(AssertUnwindSafe(|| { + let mut v = v.iter().map(|r| Wrap(r.clone())).collect::>(); + v.retain(|w| match *w.0 { + 0 => true, + 1 => false, + 2 => true, + 3 => false, // Drop panic. + _ => true, + }); + })) + .unwrap_err(); + // Other elements are dropped when `drop` of one element panicked. + // The panicked wrapper also has its Rc dropped. + assert!(v.iter().all(|r| Rc::strong_count(r) == 1)); +} + #[test] fn test_dedup() { fn case(a: Vec, b: Vec) {