Forbid destructors on objects that aren't 'static bounded

[pcwalton]

There is a problem with destruction of arenas. Objects in an arena can freely point to other objects in the arena. While an object is being destroyed, it can therefore access any other arbitrary object in the arena, including objects that were already destroyed. This can cause badness.

To me the easiest fix seems to be to require that objects with destructors be 'static bounded.

cc @nikomatsakis

[huonw]

Should #[unsafe_destructor] also override this (like it overrides the Send restriction) and allow non-'static objects to have destructors?

[pcwalton]

Yeah, #[unsafe_destructor] seems like the right escape hatch.

Maybe we should have two traits: Drop and UnsafeDrop, where the drop method on the latter is marked unsafe?

[alexcrichton]

There's a real problem with using #[unsafe_destructor] in that no one understands why the destructor is unsafe. The compiler just says "use this attribute" and everyone goes ahead and uses it without understanding what's actually unsafe or what could be unsafe.

I think we should either thoroughly document and improve the error message about unsafe destructors, try to not have unsafe destructors, or possibly some other route (a bound indicating the flavor of destructor?).

How would UnsafeDrop work? Would you be forced to drop the value in an unsafe block?

[alexcrichton]

Also, if we're pushing for smart pointers, we're basically saying that all smart pointer (and their borrowed variants) all have to have unsafe destructors (which seems a little unreasonable?).

[huonw]

It doesn't seem too unreasonable to me. The destructors can be unsafe, if implemented incorrectly. The syntax could be a little different, but it seems very similar to an unsafe { ... } block (which almost all smart pointers need too).

[pcwalton]

I think UnsafeDrop would be the same as Drop, except that it avoids using an attribute and actually uses the unsafe keyword for easier greppability.

I'm not sure that's unreasonable. Smart pointer destructors are nearly all unsafe anyway, as they deal with memory.

[alexcrichton]

I guess I'm also worried about all destructors in general. RAII is "the way to go", and RAII very frequently has borrowed pointers, so if the "official sanctioned way" includes using unsafe code, it seems like we've made a mistake?

I may be worried about nothing, but I'm concerned about the number of #[unsafe_destructor] annotations there are today, and how frequent it will continue to be in the future.

[pcwalton]

I don't know of any alternative off the top of my head, although maybe there is some. How frequent are destructors on types with references?

Arenas are very useful and it'd be a shame to give them up.

[pcwalton]

A quick grep through the source code finds the following destructors that are using lifetimes:

• Smart pointers using #[unsafe_destructor]: Ref, RefMut
• Others that already had #[unsafe_destructor]: LittleGuard, select::Handle, condition::Guard, LocalIo, Finallyalizer, StatRecorder

So there would be no additional #[unsafe_destructor] annotations required.

[glaebhoerl]

This seems like a conservative approximation of #8861?

[nikomatsakis]

It is already supposed to be the case that one cannot define Drop
except on types that are Sendable (which implies 'static). Per the
Rust meeting some time ago when we discussed this, I had hoped to lift
this restriction to say that a type can only have a destructor if it
refers to regions that are strictly greater than its own lifetime (to
avoid cycles). However, before doing that work I've been focusing on
#3511 because in order to do that we have to have a better definition
of what the lifetime is for each value =). I'm not sure if we opened
a bug on that plan or if it was only discussed in the meeting.

There are a lot of RAII applications however that really rely on
having more flexible destructors: for example, being able to acquire a
lock and so on. If we cannot create more flexible destructors, we will
certainly require once fn support instead.

[nikomatsakis]

Ah, #8861 is the "new destructor semantics" I was speaking of.

[nikomatsakis]

On Wed, Jan 08, 2014 at 08:45:18AM -0800, Alex Crichton wrote:

I may be worried about nothing, but I'm concerned about the number of #[unsafe_destructor] annotations there are today, and how frequent it will continue to be in the future.

I 100% agree. unsafe_destructor should almost never or never be used,
the fact that we are using it so much indicates that the current rules
are broken.

The reason I say that an unsafe destructor should never be used is
that it is only sound if you can control where all instances of the
type with the destructor are used -- for example, if some type T has
an unsafe destructor, and then I put the T into a Gc, now the
destructor can run at an arbitrary time. If T can access borrowed
pointers, they may no longer be valid.

A sound case for unsafe destructor is some private RAII type that is
only used in a specific circumstances and is never placed into managed
memory.

These thoughts were the motivation for #8861.

[pcwalton]

Closing as dupe of #8861.