Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document guarantees around drop and panicing #348

Open
RalfJung opened this issue May 30, 2018 · 10 comments

Comments

@RalfJung
Copy link
Contributor

commented May 30, 2018

Currently, when a panic occurs during a regular (i.e., non-unwinding) drop of a local variable, the remaining local variables are still going to have their drop called as usual: https://play.rust-lang.org/?gist=1e2b47513bbfebb71cb6d4e05eca822d&version=stable&mode=debug

Furthermore, if a panic occurs during the drop of a struct with members, the member's drop is also still going to be executed: https://play.rust-lang.org/?gist=69c5e0e922f96dbdc939dad6453ecc6c&version=stable&mode=debug

It would be nice to have such guarantees spelled out explicitly somewhere. Given that this is externally visible behavior, it is probably covered by the stability guarantee, but still -- these guarantees will be really important for providing safe stack pinning APIs.

I am not sure what would be a good place for this to be documented, maybe somewhere in https://doc.rust-lang.org/stable/reference/?

Cc @pythonesque

(Moved here from rust-lang/rust#50765)

@tmandry

This comment has been minimized.

Copy link

commented May 15, 2019

Leaving a link here to an assumption we make in generator optimizations:
rust-lang/rust#60840 (comment)

@RalfJung

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

Thanks! I feel this is somewhat separate; your assumption is more on the level of MIR semantics that drop shim generation. But it's probably sufficiently closely related.

For a different discussion, see rust-lang/rust#60611 and rust-lang/rust#60822.

@RalfJung

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

@gnzlbg

This comment has been minimized.

Copy link
Contributor

commented May 25, 2019

@gankro

would especially appreciate if someone could properly write down [...] what on earth happens with double-panics and panic-in-destructors.

playground:

struct T(A, B, ManuallyDrop<C>);
impl Drop for T {
    fn drop(&mut self) {
        let _d = D;
        panic!();
        unsafe { ManuallyDrop::drop(&mut self.2) };
    }
}

struct U(T);

fn main() {
    std::panic::catch_unwind(|| {
      let _e = E;
      let _f = F;
      let _t = U(T(A, B, ManuallyDrop::new(C)));
    });
    // x
}

prints:

<D as Drop>::drop()
<A as Drop>::drop()
<B as Drop>::drop()
<F as Drop>::drop()
<E as Drop>::drop()

That is, when a destructor panics:

  1. The values in the Drop::drop stack frame that panics are dropped in reverse order.
  2. The fields of the type whose Drop::drop panics are then dropped in struct field order.
  3. The panic escapes Drop::drop, and unwinding continues

The type whose Drop::drop impl droped is left in a partially-dropped state. All of its fields have been dropped, but its Drop::drop impl does not finish running. E.g. in the case above, the destructor of the ManuallyDrop field was not called. All types containing it are also therefore in a partially-dropped state. Attempting to use these types in this state without re-initializing them is probably going to lead to UB.

We do not guarantee that program execution will reach // x above, e.g., if panic=abort, or if the user unplugs their computer. We do however guarantee that , if program execution reaches // x above, then all these destructors have run in precisely this order, and unsafe code can rely on that to build safe abstractions.

Doing a longjmp to a setjmp deallocates all intermediate stack storage without calling destructors. Such an API is only safe as long as it does not break unsafe code that relies on this. This is definitely the case if, when replacing longjmp with panic!, and setjmp with catch_unwind, no destructors would be run during unwinding between the panic and that catch_unwind. This is the case if those stack frames contain no fields that implement Drop. There is no way to build an abstraction that makes sure that this is the case, so in current Rust, a safe API over setjmp/longjmp is not possible. Those using them need to manually inspect the stack frames involved and make sure that they contain nothing that would call Drop::drop during unwinding.


For double panics, panic! has code like this:

if already_panicking() { 
  abort(); 
}

That is, we guarantee that if a program panic!s while the stack is being unwinded, the program aborts. Unsafe code can rely on this guarantee to build safe abstractions as well.

@RalfJung

This comment has been minimized.

Copy link
Contributor Author

commented May 31, 2019

Also see the discussion at rust-lang/rust#60766 (comment). Though that's actually more UCG territory?

@gnzlbg

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

I ran into this today, and if we are dropping a struct like:

struct H(A, B, C, D, E, F, G);

and dropping say field F always panic (playground), all fields except for F are successfully dropped.

If dropping two fields panic (playground), then we get a double panic.

This obviously extends to slices (one panic and double-panic).

@RalfJung

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2019

That seems exactly as expected? What is interesting here?

This behavior matches exactly what happens when you have multiple locals (let a; let b; let c;) and one of their drops panics.

@gnzlbg

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

That seems exactly as expected? What is interesting here?

Well, in hindsight, the current behavior makes perfect sense and is consistent with the rest of the language.

This came up while reviewing a PR that modifies Vec::truncate(N) which is implemented with different semantics (rust-lang/rust#64375).

@RalfJung

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2019

Well, in hindsight, the current behavior makes perfect sense and is consistent with the rest of the language.

Except for the order, of course. Structs are dropped first field first while locals are dropped last local first.

This came up while reviewing a PR that modifies Vec::truncate(N) which is implemented with different semantics (rust-lang/rust#64375).

I see. Yes, I can imagine many of our collections would not be quite as careful as the compiler-generated drop glue is.

@gnzlbg

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

Except for the order, of course. Structs are dropped first field first while locals are dropped last local first.

Yes. By consistency I meant that all aggregates are dropped first field first (both structs and arrays), but I know you call all aggregates structs so all is fine 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.