Drop order of non-`Drop` types

help
1

I am confused by how adding impl Drop for Foo breaks the following code snippet:

use std::fmt::Debug;

#[derive(Debug)]
struct Foo<'a>(Option<&'a u32>);

impl<'a> Drop for Foo<'a> {
    fn drop(&mut self) {
        println!("{self:?}")
    }
}

fn main() {
    let mut foo = Foo(None);
    let a = 1;
    foo.0 = Some(&a);
}

error[E0597]: `a` does not live long enough
  --> src/main.rs:15:18
   |
14 |     let a = 1;
   |         - binding `a` declared here
15 |     foo.0 = Some(&a);
   |                  ^^ borrowed value does not live long enough
16 | }
   | -
   | |
   | `a` dropped here while still borrowed
   | borrow might be used here, when `foo` is dropped and runs the `Drop` code for type `Foo`
   |
   = note: values in a scope are dropped in the opposite order they are defined

I am aware of the reverse drop order. I also understand that it is not desirable for the compiler to automatically reorder the destructors (perhaps toplogically) because side effects of drop() may rely on the deterministic drop order for correctness.

I read the reference like this:

  • if T: Drop, then the compiler implicitly adds a call to T::drop() at the end of the scope, in the order described.
  • if T is not Drop, it's a no-op, the value just goes out of scope, and the memory will eventually be overwritten.

So I would have expected that the borrow checker see this code:

fn main() {
    let mut foo = Foo(None);
    let a = 1;
    foo.0 = Some(&a);
    // drop(a);  // not actually called, since Option<u32> is not Drop
    drop(foo);
}

This would have borrow-checked fine!
But the error message shows that this is not what the borrow checker sees. It clearly sees an actual drop of a.

I have a hard time building a mental model of why the drop order of non-Drop types only matters when there's another Drop type.

1 Like
2

Even though Option<u32> does not contain drop glue, and hence no code is run for dropping it, it still goes out of scope at this point, invalidating all references to it.

3 Likes
3

Interesting question. It’s certainly something that the borrow checker, and drop-check, doesn’t consider at the moment. Even though there’s some logic as to the effects of custom Drop implementations vs. the automatic ones, these effects are:

  • limited to insights about what values a non-custom implementation can’t access; and
  • don’t include insights about how “valid” the self value may still be after an automatic implementation that’s actually “doing nothing”.

It’s interesting that in a different situation, the latter kind of insight can play a role. For instance in the rules governing constant static promotion: Literals, and certain types of const-expressions of type u32 permit &expr to be lifeted to &'static u32, and the completetely-“no-op” nature of the drop glue of u32 does play a role in determining this to be a legal transformation without altering program behavior in those cases where the longer lifetime wasn’t actually required.

But for borrow-checking around Drop code, variables are still always assumed to go out of scope precisely where they get dropped, and not any later.

This means that – without your custom impl Drop for Foo…, the code works fine, since a does go out of scope first, but the automatic drop glue of Foo<'a>, and of Option<&'a u32> and &'a u32 does not possibly access the target of the &'a u32 anymore, so it’s fine if that target was already considered “dead” and out-of-scope.

In principle, if that helps at all, you can consider

let mut foo = Foo(None);
let a = 1;
foo.0 = Some(&a);

equivalent to

let mut foo = Foo(None);
{
    let a = 1;
    foo.0 = Some(&a);
}

which (in my mind) would make it at least a little bit more weird if the custom drop code of Foo (by means of accessing the &u32’s target) could still read the memory of a outside of this block.

If you’re still feeling fine with the above latter code, it’s still not entirely clear, in my opinion, where to draw the line…

E.g. how about

let mut foo = Foo(None);
{
    let a = 1;
    foo.0 = Some(&a);
}
some_other_operation();

Or how about

let mut foo = Foo(None);
{
    let a = 1;
    foo.0 = Some(&a);
}
do_something(&foo); // <- this could still access `a`, too

and – in case that feels any different – you could also imagine the same situation using some let mut a and Foo(&'a mut u32), in which case access to a after its syntactic scope ended could even mutate.

One thing that the current rules would in principle allow is for the memory of a on the stack to be re-used after it has gone out of scope. This means for example, the function call to the custom Drop implementation of Foo could itself be given a stack frame that overlapped with the memory previously occupied by a. I’m not sure if this is something rustc (together with llvm) will actually be able to make use of or not; but even if not, they might do it in the future.

So this is a concrete way in which “a goes out of scope” can have a practical effect[1] even though dropping a did not involve calls to any custom destructor logic.

  1. and this effect would turn the rejected code here into a truly problematic “use after free” kind of situation; printing some garbage value, assuming some local stack data that the println infrastructure is setting up would overwrite the previous value of a ↩︎

2 Likes
4

The 2 bullet point happens if T is not Drop. Otherwise, for example, a String field would never be deallocated unless you implemented Drop. String itself does not implement drop, in fact, and instead also relies on this "drop glue".

What mostly matters to the borrow checker is what the drop implementation (including drop glue) is known or allowed to observe. On stable Rust -- when you write your own Drop -- you are allowed to observe everything about your type. That means, for example, the borrows related to your type must still be active when your type drops. Which allows you to do things like print them, as in your playground. (The borrow checker doesn't look at your method body, but assumes you may observe everything about your type.)

Compiler generated drop glue is known not to observe fields, other than recursively dropping them. If the end result is that there is no actual destructor, going out of scope is mostly a no-op. For example, the Option<&u32> going out of scope is known not to observe the reference (and references going out of scope do not observe their referent). The borrow checker then recognizes that the borrow of the u32 can end before the Option drops.

Foo doesn't have a way to opt in to that behavior -- to promise not to observe the borrowed value. But there's is also an unsafe, unstable way to declare "my Drop won't do anything with this generic other than drop it" and the like, which many std types use. This allows for the drops of types which are not no-ops to be treated more like trivial drops by the borrow checker, so that a Vec<&u32> acts more like a Option<&u32>, say.

In summary: the drop order doesn't change, but what borrows need to stay active because they may be observed can change.

I said mostly a no-op when values with trivial destructors go out of scope because it's still an error for values which are themselves borrowed to go out of scope. The way I prefer to think of this is that the value becomes undefined. And, for example, it's not allowed to have a reference to an undefined value.

Note that there, you are calling std::mem::drop, which is not magical -- it's just a way to move a value to some place where it immediately goes out of scope. There is no way to call Drop::drop directly, and it wouldn't include the full destructor functionality (drop glue) anyway.[1] And it behaves differently from a borrow-checking perspective too -- it will keep some borrows active when going out of scope would not have, due to the special "can borrows be observed when this drops" considerations discussed above.

  1. drop_in_place is how one can manually invoke a destructor. ↩︎

3 Likes
5

Thank you all. This is immensely helpful.

Yeah, I remember seeing this 'desugaring' mentioned somewhere in the nomicon, and I was equally confused how the borrow checker would possibly allow this when I mentally substitute some_other_operation(foo) with the drop glue.

I understand what @steffahn and @quinedot are saying is that the borrow checker is capable of some level of understanding of auto-generated destructors, in particular that they will not observe any fields, and thus that it's actually fine if that field is already dead. On the other hand, a custom implementation follows the usual borrow-check rules that conservatively stop at the function signature (and never actually look at the implementation).

Following up:

Are there actual correctness (or soundness) concerns for the drop order of non-Drop values? Could the compiler in principle be made to reshuffle them into topological order without stepping on people's toes? Such that my original example compiled in either case.

6

One principle that does currently apply to borrow checking is that it is a compilation step that’s not affecting runtime behavior of the program.

This has some benefits:
  For example, users that want to understand their program’s behavior do not need to understand the precise behavior of borrow checking.
  It’s easier to upgrade the borrow checker to become “more capable” (accepting more non-problematic programs) in the future if the only “effect” that borrow-checking may have on a program’s behavior is that compilation can fail (and a future upgrade would make it fail less often).
  Also, it can probably improve the performance of borrow-checking if it’s only a check, and not some sort of computational step that needs to produce a more complex output (based on which this kind of “topological order” could be created).

Considering effects restricted to drop order of “non-Drop” values, the usability concerns are probably minimal to non-existant. If the only negative effect can be e.g. slighly more stack usage, that’s not really something a user needs to reason about. Drop order is complex and often not really relevant for the programmer anyway.

Still, the restriction to “non-Drop” values (even though, as I have mentioned for the case of constant static promotion, such a restriction does exist for another language feature already) could still be a negative… making types with drop glue more of a second-class citicen in the language… causing more possibility for compilation errors that have a very non-obvious cause (introducing a custom Drop resulting in a borrow-checking error[1])… Probably there’s also more cases then of inadvertent somewhat-semver-breaking changes to API surfaces, if for example someone converts their struct Error { code: i32 } to something like struct Error(Box<Inner>).

One idea that IMHO could be a nice to help your concrete use case: Ideally some heuristic in the compiler could somehow catch most easy cases such as yours, where compilation is only hindered by wrong relative drop order of variables with otherwise-identical[2] scope. Then the compilation error could come together with a help message that suggest the appropriate call to drop(foo) to be added to solve the issue, or alternatively, it might suggest an appropriate early declaration of let a;. This way, you could get an “it works after auto-fixes” kind of situation, whilst:

  • future upgrades to the borrow checker can be independent from such heuristics, or the heuristics are also allowed to become worse in certain cases, so this isn’t inhibiting future develoment on rustc and the borrow checker
  • any potential performance-impact to have borrow-checking (or the additional heuristics in question) be able to provide some sort of “output” won’t be relevant, since the compiler only needs to produce these suggestions once, and afterwards it would be part of the code, and your on the “happy path” again
  • users can more easily reason about altered drop orders in cases when they do care about them, if they are still indicated in code
  • the heustics can hence also suggest fixes in cases with types that aren’t non-Drop, because the programmer can still review them and notice the (presumably more uncommon) case where a different drop order wasn’t what they wanted[3]

  1. I do acknowledge that your example is also a case of “adding impl Drop causes new borrow-check error”, and that particular case would be solved.
    But arguably, this might be a “less bad” case. At least here, it was the introduction of a Drop impl *to a type containing a lifetime. It’s less surprising that that’s relevant to borrow checking than – say – the effect of adding a Drop impl to something as harmless as u32 would be. (Let’s ignore that that’s impossible due to Copy; I’m merely referring to u32 because that’s the type where Drop impl would then matter in your concrete example. ↩︎

  2. or near-identical… or maybe even a heuristic with more general applicabiliy will turn out useful :man_shrugging:t2: ↩︎

  3. this isn’t limited to adverse affects from the changed drop order directly

    It’s also reasonable that an error message like this may come up in cases where e.g. the code that put the reference to a into foo had a bug, and it was meant to be a reference to a different variable; so it could indirectly fix bugs to keep the basic rules more simple and leave the programmer involved in changes to drop orders. ↩︎

1 Like
7

Oh, looking at it from that angle made the whole thing click for me! I now also understand better why the error came about in the first place.

Thanks a lot!

8

Of course. E.g. consider a MutexGuard, which unlocks the mutex on drop. Whether the mutex is locked or not can certainly affect your program correctness. It's hard to give an example with the usual use of Mutex which doesn't run afoul of borrow-checking issues, but note that a mutex could be systemwide-allocated, and perform synchronization with other processes. In that case the actual resource protected by the mutex likely won't be guarded by it at the type system level.

The drop code could do anything: commit or rollback database transactions, do network requests, call arbitrary FFI code. This means that reordering drops can have arbitrary effects, unless the compiler can prove otherwise.

9

But that's precisely the opposite: that's very-very Drop type!

The question was bout non-Drop types!

10

The OP was about a type with explicitly implemented Drop, so that's covered. That also covers types with Drop glue. It isn't relevant whether the type explicitly implements Drop or just calls recursive Drop on its fields, either case can have side effects. And in case where the type has no Drop glue at all, the answer is "the compiler already knows that there are no side effects and does reordering, as much as possible".

11

The statement in question, “Are there actual correctness (or soundness) concerns for the drop order of non-Drop values?”, did refer to the type Option<u32> of the variable a.

The point of the previous discussion was exactly that this is not the case :wink: At least considering the possible consequence of: “allowing a of type Option<u32> to be considered to be dropped later than foo in the OP’s code example”, which doesn’t happen. Of course foo in turn does have drop glue, but if we’re only considering destructor code, we could stil argue: one of two is a no-op, they can still be re-ordered.

12

And that is where @afetisov example becomes relevant: data item that MutexGuard is guarding is, very often, something simple: i32 or even bool… and yet it would be very wrong to make it possiblke to access that data after Drop for MutexGuard is called.

The whole point of MutexGuard is to stop such possibility — even if the data in question is simple, non-Drop one.

That's why Drop is moved around only if both variables don't have a Drop glue.