Skip to content

Commit

Permalink
Rebuild BinaryHeap on unwind from retain
Browse files Browse the repository at this point in the history
  • Loading branch information
dtolnay committed Jan 15, 2023
1 parent 0d3eaa8 commit fa2ff4d
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 7 deletions.
24 changes: 18 additions & 6 deletions library/alloc/src/collections/binary_heap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -851,18 +851,30 @@ impl<T: Ord> BinaryHeap<T> {
where
F: FnMut(&T) -> bool,
{
let mut first_removed = self.len();
struct RebuildOnDrop<'a, T: Ord> {
heap: &'a mut BinaryHeap<T>,
first_removed: usize,
}

let mut guard = RebuildOnDrop { first_removed: self.len(), heap: self };

let mut i = 0;
self.data.retain(|e| {
guard.heap.data.retain(|e| {
let keep = f(e);
if !keep && i < first_removed {
first_removed = i;
if !keep && i < guard.first_removed {
guard.first_removed = i;
}
i += 1;
keep
});
// data[0..first_removed] is untouched, so we only need to rebuild the tail:
self.rebuild_tail(first_removed);

impl<'a, T: Ord> Drop for RebuildOnDrop<'a, T> {
fn drop(&mut self) {
// data[..first_removed] is untouched, so we only need to
// rebuild the tail:
self.heap.rebuild_tail(self.first_removed);
}
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion library/alloc/src/collections/binary_heap/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,9 @@ fn test_retain_catch_unwind() {
});
}));

assert_eq!(heap.into_vec(), [1, 2]); // BAD!!
// Naively this would be [1, 2] (an invalid heap) if BinaryHeap delegates to
// Vec's retain impl and then does not rebuild the heap after that unwinds.
assert_eq!(heap.into_vec(), [2, 1]);
}

// old binaryheap failed this test
Expand Down

0 comments on commit fa2ff4d

Please sign in to comment.