Pre-RFC: Leave auto-trait for reliable destruction

I just created a "safe-leaks crate" which demonstrates different ways that data can be leaked in safe Rust today. It's also a challenge! If you know any other techniques, feel free to send a PR :slight_smile:

No easy way to test, but if you allow 'static values as well there's a few more techniques that can be used - including passing ownership of the value to a different thread and letting that thread spin/be parked until the end of the program (at which point the thread will be killed without dropping anything).

1 Like

Thanks for writing this proposal! I include an old thread from the Rust users forum which I find relevant:

As was mentioned here

this is not possible for Drop types. As long as Leave was a supertrait, this was fine because !Leave guaranteed that the type wasn't Drop; but for the new, separate ImplicitDrop something else needs to be thought of.

This is dangerous. The original scoped_thread problem was UB in safe code; if you have a safe destroy which "leaks" the guard, you're back at square one. This means IMO that destroy should be unsafe; which means that a new mechanism for safely (edit: or not, see below) destroying the type in the correct context should be added.

Which leads to the next question: who should be destroying the type? In most examples it says

and I think this is the correct direction, but the question is, what makes this function "special" and allows it to destroy the type? In the destructuring case, one could rely on private fields to make sure that only methods can do so, but if a type is Drop this doesn't work.

One answer is, this is inherently unsafe, and one should just use the (unsafe) destroy method. This certainly works for the unsafe cases, such as scoped threads, which need unsafe anyway, so it makes sense that the method which destroys the handle does it unsafely. And for cases without Drop destructuring can still be used. But maybe that's not good enough. Any good examples of useful safe Drop impls?

Allow destructuring of structs that implement both Drop and !ImplictDrop?

That's probably not bad. I was going to suggest (following a discussion with @realcr) something along the lines of "whatever scope is allowed to access non-pub-s", but wasn't sure what's a good way to do this that doesn't seem too magical.

One way I thought of was to add a "private" method called something like explicit_drop to any type which is !ImplicitDrop, but that's ugly, magical, and difficult to find (could be somehow documented in ImplicitDrop).

But another way to do this is to allow what you're suggesting and then the "privateness" issue is returned to the implementor and private fields.

Speaking of private fields: what about enum-s? Anyone can destructure them, which might be a problem for any !ImplicitDrop enum, even if it doesn't implement Drop. Is the answer "we don't care, just wrap it in a struct" like in many std errors?

1 Like

!ImplicitDrop is meant as a control flow lint, its purpose is not safety. !Leak OTOH is about safety, and would be needed by a scoped task in order to safely allow static borrowing. Apologies for the confusion.

That said, a guard type such as TaskHandle or MutexGuard could benefit from !ImplicitDrop too, but that's for the purpose of avoiding programmer error:

let foo = "hello".to_string();
let task = ScopedTask::spawn(async {
    println!("{}", &foo); // Static borrowing possible (!Leak)
});
something_else()?; // Error: task possibly dropped here (!ImplicitDrop).
task.await; // This is the preferred way of consuming the task.

Sorry, I missed this - and thanks for catching this error. This needs further thinking. I'll edit the proposal.

One answer is, this is inherently unsafe, and one should just use the (unsafe) destroy method. This certainly works for the unsafe cases, such as scoped threads, which need unsafe anyway

Again, scoped threads and tasks should not need unsafe with !Leak. In either case, providing safe APIs - all else equal - is enormously useful. Besides, destroying before reaching a terminal state could have different behaviors for different types:

  • Do nothing/log an error (e.g. an async buffered file that should be flushed as a last step)
  • Poison some shared resource (e.g. an async Mutex didn't finish its critical section)
  • Block the thread until it is safe to continue (e.g. the JoinHandle of a scoped thread)
  • Abort the process if it's impossible to continue in a safe & sound manner (e.g. an T: !ImplicitDrop container reaching a state where the nested destructors T can't be invoked)

...allow writing impl !ImplicitDrop for struct-s only?

enum-s would still become !ImplicitDrop if any field is !ImplicitDrop
but it would be an error to declare this yourself

Thanks for the clarification of the difference between ImplicitDrop and Leak! This means that the first is relevant when there's something returning from cleanup which we don't want our users to ignore (e.g. error-handling, or a task), and the second is really about the old issue of leaking guards.

I would say that in this case, there's no real reason to create a new std::mem::destroy method, std::mem::drop is good enough: you can't call invoking this method "implictly" dropping, it literally says that you're dropping the value! This will require "blessing" std::mem::drop though (the other option will required blessing the new std::mem::destroy, so it's the same cost).

It's not that it invokes drop on destructuring, it's a compiler error:

struct Bar;
struct Foo(Bar);

impl Drop for Foo {
    fn drop(&mut self) {
        println!("dropped")
    }
}

fn main() {
    let foo = Foo(Bar);
    let Foo(x) = foo;
}
error[E0509]: cannot move out of type `Foo`, which implements the `Drop` trait
  --> src/main.rs:12:18
   |
12 |     let Foo(x) = foo;
   |             -    ^^^ cannot move out of here
   |             |
   |             data moved here
   |             move occurs because `x` has type `Bar`, which does not implement the `Copy` trait

This means that the cleanup method also needs to call std::mem::drop explicitly and can't destructure, which is unfortunate, but maybe not terrible. I must admit I'm not quite sure what's the reason destructuring Drop structs is forbidden; why moving out of them is forbidden is clear (you can't leave the struct partially uninitialized, since drop needs to be called at the end), perhaps it's too difficult to assert that this is a full destructuring? There do seem to be workarounds today, (e.g. derive_destructure)).

Though why couldn't ImplicitDrop be a supertrait of Drop?

1 Like

alternatively to the above, couldn't

std::mem::drop

become the magic function for destroying !ImplicitDrop instances?

  • destruction code would sit same as before in a Drop impl
  • both Drop and !ImplicitDrop would be implemented
  • users would be forced to use std::mem::drop to get rid of the instance

Because

Yes, that's exactly what I suggest here:

In the original message I confused the requirements of ImplicitDrop and Leak, leading to the conclusion that the general library function has to be unsafe, and we might want a safe alternative for the library author. Since that's not the case, I agree: std::mem::drop should be that "magic" function.

The ability to destructure droppable objects – either instead of dropping them, or from a Drop impl itself – is one of those things that's been proposed over and over, but has never made it across the finish line. In case anyone wants to fish for ideas:

7 Likes

It seems the following change

  • add !Leave and !ImplicitDrop auto-traits
  • allow !ImplicitDrop to be disposed off in two ways only
    • panic unwind
    • std::mem:drop (which thus becomes magic)
  • make Leave and ImplicitDrop default trait bounds unless opted out of

could be done stand-alone, without any other change to the standard library or the whole ecosystem.

You won't be able to put !Leave or !ImplicitDrop objects into Vec nor into Rc nor use them as a generic parameter anywhere. But that's ok - these are very specialized types. Normally you'd only hold them on stack - and nothing else :slight_smile:

If pressure builds up later to add ?Leave or ?ImplicitDrop anywhere in std there will be time and space to carefully consider such backwards-compatible decisions. Also if there is much need some specialized crates may show up geared specifically to provide collections for these specialized objects etc.

Incidentally !ImplicitDrop doesn't seem to help much with futures. (run_to_completion_futures seems to help instead). But it looks like !ImplicitDrop can add clarity into the usage of RAII guards in general.

I think there's another use case for such a proposal: static-rc by @matthieum.

This crate has a type StaticRc<T, const NUM: usize, const DEN: usize>, which holds ownership of NUM parts of a T on the heap. To drop it without memory leak, it is necessary to join all parts together so that NUM==DEN. Since it's not possible to prevent dropping, there's no way to check it at compile time.

My recent proposal rdylib (a variation of dylib that allows unloading DLLs), also requires properly handling of RAII.

What scoped threads/scoped tasks and unloadable DLLs all require is the concept of relative 'static. A scoped thread has it own lifetime, which is not 'static and Arc need to be customized for this to avoid being leaked. The same happen to scoped tasked. And if a DLL is unloadable, every symbol lives in that memory space should not outlive the DLL, so the DLL's 'static is also not the same as the host program's.

Therefore I see this proposal as one step towards the solution for relative 'static.

Do we have any chances to have this in the next edition?

Leak auto trait is really great accomplishment.

The cutoff for ideas on what to include in the 2021 edition has already passed.

What's about:

NoDestroy implied auto trait - with the purpose of ImplicitDrop.
We don't implement it for any type that implement a trait Destroy:

impl<T: Destroy> !NoDestroy for T {}

Destroy trait:

/// A trait for finalization, with support for data exchange
trait Destroy: ?Sized {
    type FinalArgument = (); //what is required for destroy
    type LeftOver = (); //what is left after destroying an object
    fn destroy(self,a: <Self as Destroy>::FinalArgument) -> <Self as Destroy>::LeftOver;
}

Edit: and also I think it would be better to forbid implementing Drop for Destroy types.

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.