What to do if the destructor of a pinned field panics?

[Diggsey]

Let's say I have an enum, with a variant containing some structurally pinned fields:

#[pin_project]
enum Foo {
    A {
        #[pin] pinned_field1: X1,
        #[pin] pinned_field2: X2,
        #[pin] pinned_field3: X3,
        unpinned_field: Y,
    },
    B,
}

Given a Pin<&mut Foo>, I would like to do something like mem::replace(pinned_foo, Foo::B).

Now, in general this would not be allowed, because it would move the pinned fields. However, I should be able to drop the pinned fields in-place, take the unpinned field, and then overwrite the entire value with Foo::B.

One way of doing this is just using the relevant ptr::{read, write, drop_in_place} methods. The problem is that the destructors of the pinned fields might panic, in which case it's unclear what I would need to do to avoid causing UB.

Firstly, if the destructor of a pinned field panics, am I allowed to reuse that memory ever? The contract for Pin says that the destructor must run before the memory is reused, but it doesn't say whether it has to complete successfully.

From what I can tell, the optimal method would be to create structs like this:

struct DropInPlaceHelper<T>(*mut T);

impl<T> DropInPlaceHelper<T> {
    fn drop(&mut self) {
        ptr::drop_in_place(self.0);
    }
}

struct OverwriteHelper<T>(*mut T, MaybeUninit<T>);

impl<T> OverwriteHelper<T> {
    fn drop(&mut self) {
        ptr::write(self.0, self.1.assume_init());
    }
}

And then construct instances of it on the stack for each field:

fn partial_replace(ptr: *mut Foo, new_value: Foo) -> Y {
    let _helper0 = OverwriteHelper(ptr, MaybeUninit::new(new_value));
    let res = ptr::read(&mut (*ptr).unpinned_field);
    let _helper1 = DropInPlaceHelper(&mut (*ptr).pinned_field1);
    let _helper2 = DropInPlaceHelper(&mut (*ptr).pinned_field2);
    let _helper3 = DropInPlaceHelper(&mut (*ptr).pinned_field3);
    res
}

1. Would this be valid?
2. Is there a better/more ergonomic way of doing this? (that doesn't require changing the enum definition)

[comex]

Firstly, if the destructor of a pinned field panics, am I allowed to reuse that memory ever?

Yes, or else stack pinning is broken as a whole. Consider a function that pins a variable on the stack but doesn't do anything else unsafe. If the destructor of that variable (or a field of that variable, doesn't matter) panics, the stack frame will still go out of scope as part of unwinding. If some function earlier in the call stack catches the panic (also a safe operation) and then calls more functions, those functions' stack frames can end up using the same memory as the one that went out of scope.

[Diggsey]

I created a simplified version with a single pinned field, and ran it though MIRI:
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=e1205fe7dab0a1cf8d26f6ade4c2b83c

As you can see, MIRI gives an error:

error: Undefined Behavior: trying to reborrow for SharedReadOnly, but parent tag <3975> does not have an appropriate item in the borrow stack
   --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/ptr/unique.rs:116:9
    |
116 |         &*self.as_ptr()
    |         ^^^^^^^^^^^^^^^ trying to reborrow for SharedReadOnly, but parent tag <3975> does not have an appropriate item in the borrow stack
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
            
    = note: inside `std::ptr::Unique::<Map<Foo, [closure@src/main.rs:89:44: 89:56]>>::as_ref` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/ptr/unique.rs:116:9
    = note: inside `alloc::alloc::box_free::<Map<Foo, [closure@src/main.rs:89:44: 89:56]>>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/liballoc/alloc.rs:282:28
    = note: inside `std::intrinsics::drop_in_place::<std::boxed::Box<Map<Foo, [closure@src/main.rs:89:44: 89:56]>>> - shim(Some(std::boxed::Box<Map<Foo, [closure@src/main.rs:89:44: 89:56]>>))` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/ptr/mod.rs:180:1
    = note: inside `std::intrinsics::drop_in_place::<std::pin::Pin<std::boxed::Box<Map<Foo, [closure@src/main.rs:89:44: 89:56]>>>> - shim(Some(std::pin::Pin<std::boxed::Box<Map<Foo, [closure@src/main.rs:89:44: 89:56]>>>))` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/ptr/mod.rs:180:1
note: inside `main` at src/main.rs:94:1
   --> src/main.rs:94:1
    |
94  | }
    | ^
    = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:67:34
    = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:52:73
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<[closure@std::rt::lang_start_internal::{{closure}}#0::{{closure}}#0 0:&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/sys_common/backtrace.rs:130:5
    = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:52:13
    = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{{closure}}#0 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:297:40
    = note: inside `std::panicking::r#try::<i32, [closure@std::rt::lang_start_internal::{{closure}}#0 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe]>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:274:15
    = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{{closure}}#0 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panic.rs:394:14
    = note: inside `std::rt::lang_start_internal` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:51:25
    = note: inside `std::rt::lang_start::<()>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:67:5

I would expect this program to have well-defined behaviour.

[RustyYato]

The call to self.project() invalidates self_ptr. So no, this doesn't follow stacked borrows. You cannot use pin_project. In fact, if you remove the call to pin_project, and go through self_ptr directly, everything works

[RalfJung]

Firstly, if the destructor of a pinned field panics, am I allowed to reuse that memory ever?

Yes. (This was asked before somewhere. We clearly need to improve the docs.)

The drop-requirement for pinning is that you must call the destructor. Once the destructor finishes, either by returning or panicking, you may reuse that memory.

[RalfJung]

Firstly, if the destructor of a pinned field panics, am I allowed to reuse that memory ever?

I brought this up for T-lang nomination in rust-lang/rust#71607.

[RalfJung]

Fixed by rust-lang/rust#71607.