Why does the Drop trait take a mutable reference?

The drop trait is currently defined as:

fn drop(&mut self) {
    // ...
}

But the thing is, after dropping the value, the value is no longer accessible.
This is also the reason why you can't call the Drop trait explicitly as then you could cause a use after free bug.
So my question is: why isn't the Drop trait defined like this so that it consumes the value?:

fn drop(mut self) {
    // ...
}
1 Like

This would be useful, but there are problems:

  1. You’d then have a value of type Self inside of fn drop(), which would then be dropped at end of scope. You would need one of:
    • Some kind of special parameter type that means “Self but without dropping”.
    • Special pattern syntax that means “destructure this type even though it implements Drop” which can be used inside of the function body to prevent recursion. (I think this would be a good feature to have, but it is another feature.)
    • Everyone implementing fn drop(self) needs to mem::forget the value.
  2. Some values (those that use !Unpin) depend on drop() running without the data being moved, but fn drop(self) implies a move.

I think it would be good if it were permitted to implement Drop with either fn drop(&mut self) or fn drop(self). There’s already interest in adding fn drop(self: Pin<&mut Self>), which would need the same language feature (though be easier to implement, since the ABI is identical).

See also Drop::pin_drop and its tracking issue 130494, which includes designs for drop(&pin mut self).

1 Like

The flip side of this is that drop is already pretty special, and it's in part just an accident where the special wound up.

error[E0040]: explicit use of destructor method
 --> src/main.rs:3:7
  |
3 |     x.drop();
  |       ^^^^ explicit destructor calls not allowed
3 Likes

I'd already started working on an RFC which let you do something like

fn drop(Self { field1, field2 }: Self) {
     // do something with field1 and field2
}

in order to implement a drop that can safely move out of the fields. (Requiring all the fields to be moved in the pattern avoids the problem of "you have a Self so it will just drop again" by never actually giving you a Self.) RFC 3738 (which @kpreid linked above) is playing in a similar area.

Note that unfortunately the syntax I suggested above isn't properly backwards-compatible (when used in contexts other than drop, which could be made a special case): in current Rust, such a pattern match attempts to copy out of every field and then (at the end of the function) drop the original object, whereas the desired behaviour is to move out of every field and not drop the original object. So you would need some sort of destructure! syntax like in RFC 3738. (But requiring a destructuring pattern as the argument to drop immediately solves the infinite recursion problem.)

On a side note, there is an unstable type DropGuard that almost allows this problem to be solved in safe code (but with very convoluted syntax – you have to make the type a wrapper around DropGuard which is in turn a wrapper around a private struct holding the type's fields). The primary problem is that you have to specify the destructor as a FnOnce and that means that, in order to store the destructor in the object (presumably as a ZST), you need to be able to name the type of the FnOnce. In present Rust, I don't think it's possible to create something that implements FnOnce and has a nameable type (although there are multiple ways to do it using unstable features, e.g. via type alias impl Trait or via unboxed_closures).

It would be possible to create a trivial variation on DropGuard that uses a different trait, in order to allow safe code to implement destructors that move out of fields of the dropped object; I was actually planning to do that, when I happened to check IRLO and saw this thread. My guess is that there are unlikely to be any "official" solutions any time soon, so a stop-gap DropGuardButWithADifferentTrait is what I'm likely to use to solve this problem for the time being. (It is a real problem that, at least for me, comes up very frequently; I normally solve it with ManuallyDrop::take but this is an unsafe function and thus means that I can't write my program entirely in safe code.)

2 Likes

Don't you get an infinite loop if you have let inf_loop = Self { field1, field2 }; in the body (without another destructure)?

That would result in an infinite loop no matter what is drop's signature.

2 Likes

Guys this is covered in like the next sentence. That's a problem for that specific syntax unless you made drop just really special. Special case syntax for only implementing drop is probably not a great idea, but there are plenty of ways already referenced that sound fine.

Yes, but I don't see why you would do that.

Even in current Rust, you get an infinite loop if you create a new Self in <Self as Drop>::drop, but there's very little reason to want to construct an object of a given type in that type's destructor.

Since Drop is already magic for being mutually exclusive with Copy, there's probably also ways here to add other kinds of drop that would be mutually exclusive in the same way.

I could imagine a

trait DoItAllYourselfDrop {
    fn drop(value: ManuallyDrop<Self>);
}

for example where you take responsibility for handling all your fields too, but in exchange get ownership in drop.

3 Likes

I almost made this in nightly, which is less magic (no mutually exclusive impls) but idk if arbitrary self types is planned to allow the pattern: Rust Playground

also idk if it is non-breaking to replace drop with this

Drop is actually dyn-compatible, so this is technically breaking ^^

Also, Drop can be implemented for DSTs, which is another issue with fn drop(self).

FWIW, I wrote quite a lengthy post about this topic and tangential ones over the documentation of

More specifically:

  1. https://docs.rs/safe-manually-drop/0.1.2/safe_manually_drop/appendix/#what-does-dropvalue-do

  2. https://docs.rs/safe-manually-drop/0.1.2/safe_manually_drop/appendix/#what-would-it-take-to-have-owned-access-in-custom-drop-glue--drop_in_place-logic

My view has always been that drop is something of a misnomer. A more descriptive name would have been finalize.

As an automatic memory management system that relies on lifetime tracking rather than reference counting, Rust already takes care of deallocation. The primary purpose of drop is therefore not to free memory, but to perform any final cleanup required before the value is destroyed.

I didn't see what I consider to be the correct answer here: calling Drop::drop is not the only thing that happens at end of scope! When a value reaches end of scope, the special std::ptr::drop_in_place function is called on it (morally). That function does two things: it calls Drop::drop if there is one, and then calls drop_in_place on the fields recursively. Imo that's the true reason Drop::drop can't take your thing by value: the value is still needed for some more cleanup.

This does however beg the question: why does drop_in_place take a reference instead of the whole value? I think it could actually (if we ignore pinning), this is a magic function already. Pinning is the clearest reason why we couldn't change that today; performance seems to be the original motivation for this signature (see Tracking issue for drop_in_place · Issue #27908 · rust-lang/rust · GitHub)

drop_in_place takes a raw pointer, not a reference. If you want something that takes the whole value there's drop, but of course that's no longer "in place".

I'm fairly sure that you could make a version that takes a reference, but it would have to have a signature of fn drop_in_place<T>(&'static mut T) -> &'static mut MaybeUninit<T> in order to be both sound and useful, and &'static mut T isn't a super-common type. (It doesn't work with a shorter lifetime in case something tries to access the dropped value after the lifetime of the reference is over.) A reference to the resulting uninitialised memory is returned because otherwise there's no way to actually do anything with the memory afterwards and you get a memory leak.

I realised recently that mutable references actually have two type parameters rather than one: the type that the referenced memory currently has, and the type that the reference is required to store in the referenced memory by the time the reference's lifetime ends (which, due to the possibility of panics, it basically has to store there at all times). Currently these types are both the same for non-'static mutable references (whereas there is no requirement on what type a 'static mutable reference stores when its lifetime ends, because its lifetime doesn't end). But splitting them up makes a lot of sense, and in particular lets you implement drop_in_place on any reference whose "bounding type" is a sufficiently large MaybeUninit.

Note that casting from &T to &MaybeUninit<T> is always fine, but &mut T to &mut MaybeUninit<T> requires correct usage of the resulting reference. &mut MaybeUninit<T> allows you to write anything into the memory.

drop_in_place takes a raw pointer, not a reference

My bad, that changed recently but only internally: the real magic function is now called drop_glue and does take a reference. Doesn't change much anyway, this is unsafe code land.

1 Like

This is why you need some proof that the memory will not be accessed again except as a MaybeUninit (such as a 'static lifetime on the reference).

Note that I'm not convinced that the cast from &T to &MaybeUninit<T> is all that sound; it allows you to call assume_init_read on the resulting reference and effectively move out of the &T, even though the &MaybeUninit<T> is initialized. (It is technically sound because the unsafe preconditions on assume_init_read call out this possibility, but it's an extra precondition that most assume_init_* don't have or need.)