Vec::retain leaks items when predicate panics

[stephaneyfx]

Since the implementation of Vec::retain changed after Rust 1.25, items may not be dropped when the predicate panics.

Reproduction example

#[derive(Debug)]
struct Foo(i32);

impl Drop for Foo {
    fn drop(&mut self) {
        println!("Dropping Foo({})", self.0);
    }
}

fn main() {
    let mut v = (0..3).map(Foo).collect::<Vec<_>>();
    println!("Before retain");
    v.retain(|&Foo(n)| match n {
        0 => false,
        2 => panic!("Boo"),
        _ => true,
    });
    println!("After retain");
}

Expected result

All 3 Foo instances are dropped. The output should look like (with Rust 1.25):

Before retain
thread 'main' panicked at 'Boo', a.rs:15:14
note: Run with `RUST_BACKTRACE=1` for a backtrace.
Dropping Foo(1)
Dropping Foo(0)
Dropping Foo(2)

Actual result

Only one Foo instance is dropped. The output since Rust 1.26 (and with 1.27.1) is:

Before retain
Dropping Foo(0)
thread 'main' panicked at 'Boo', a.rs:15:14
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Edited to clarify item leak (i.e. not dropped).

[Mark-Simulacrum]

cc @rust-lang/libs -- I believe this is allowed per stability, but we should discuss at least

[stephaneyfx]

I know that leaking memory is not considered unsafe, but I assume that the standard library strives to not leak.

[dtolnay]

Looks like the relevant change in 1.26.0 was #48065.

[alexcrichton]

Thanks for narrowing this down @dtolnay, that makes sense in that I believe the drain iterator (or similar ones) have always performed a little oddly in the face of panics in the middle. I'd personally consider this fine and would consider this ok to not consider P-high and fix in a backport.

That being said it'd of course be great to solve this!

[jonas-schievink]

Fixed by #61224 (which includes a test). Closing.