Document when we *do* guarantee that drop runs

[RalfJung]

In my understanding, we do in some circumstances guarantee that drop runs. For example:

struct Guard;

impl Drop for Guard {
    fn drop(&mut self) {
        println!("Hello!");
    }
}

pub fn test(f: impl FnOnce()) {
    let _guard = Guard;
    f();
}

Here, we guarantee that no matter the environment or whatever f does, if the stack frame of test ever gets popped or otherwise "deallocated", then the println! certainly happens. For example, f might loop forever or abort the process, but it cannot "return" or "unwind" or finish in any other way that would circumvent the printing, nor can it use longjmp to skip test's cleanup, not can it just ask the OS to outright kill the current thread (without tearing down the entire process).

(By "guarantee" I mean "we consider it okay for safe libraries to rely on this and cause UB if it gets broken" -- but there is no immediate language-level UB caused by this, so if you do this kind of skipping of destructors in a controlled way, say for your own code which you knows has nothing droppable on the stack, then you are fine.)

This is needed to actually realize the pinning drop guarantee, but it seems not to be documented anywhere explicitly?

Cc @gankro @nikomatsakis @comex

[Gankra]

I would especially appreciate if someone could properly write down in one of our primary sources (nomicon, trpl, or reference) what on earth happens with double-panics and panic-in-destructors. I feel like I've always received vague hand-waves on this matter.

[Gankra]

Note the nomicon discusses some stuff:

• https://doc.rust-lang.org/nightly/nomicon/destructors.html
• https://doc.rust-lang.org/nightly/nomicon/leaking.html

But these docs are getting extremely long in the tooth (ManuallyDrop exists now, for instance).

[RalfJung]

I would especially appreciate if someone could properly write down in one of our primary sources (nomicon, trpl, or reference) what on earth happens with double-panics and panic-in-destructors. I feel like I've always received vague hand-waves on this matter.

That would be rust-lang/reference#348. I see as as somewhat orthogonal to whether unsafe code is allowed to do things like longjmp or kill a thread without running the destructors in the stack frames.

[Gankra]

The most important thing, imo, is that the "fixed" scoped_thread pattern works:

fn main() {
  let state_to_get_borrowed_by_other_thread = ...;
  scope(|spawner| {
    for i in 0..10 {
      let borrow = &state_to_get_borrowed_by_other_thread;
      spawner.scoped_thread(move || { 
        process(borrow); 
      });
    }
  );

  // if we get here, it must be safe to do this
  drop(state_to_get_borrowed_by_other_thread);
}

// can't be bothered to work out the lifetimes here
fn scope(func: impl FnOnce(SpawnerHandle)) {
  // has a destructor that ABSOLUTELY must have run 
  // IF its borrows expire AND the thing it borrowed is still accessible.
  // Created in this closure so user can't interfere with this.
  let spawner = Spawner::new();
  func(SpawnerHandle(&mut spawner));
}

impl Drop for Spawner {
  fn drop(&mut self) {
    block_on_all_threads();
  }
}

I expect this means you cannot allow longjmp or killing a thread without unwinding. Although I'm too sick to think about this clearly.

[Gankra]

Arguably you can still "allow" this along similar lines of how I suggested permitting double-drop: implementors of Drop that are properly kept in local vars are guaranteed that their destructor will run if their var goes out of scope, but it is not language-level UB to violate this expectation. Library code is allowed to assume this doesn't happen and can cause UB if it is "skipped" though. It is up to the user of [anything like longjmp] to ensure they are only jumping over safely leakable code. Since no one will bother to document this, this basically means only jumping over your own/audited code.

[RalfJung]

The most important thing, imo, is that the "fixed" scoped_thread pattern works:

I agree, though it seems both rayon and crossbeam implement that with catch_unwind instead (and the original argument here involved whether this guarantee also applies to no_std code).

Arguably you can still "allow" this along similar lines of how I suggested permitting double-drop: implementors of Drop that are properly kept in local vars are guaranteed that their destructor will run if their var goes out of scope, but it is not language-level UB to violate this expectation. Library code is allowed to assume this doesn't happen and can cause UB if it is "skipped" though.

Absolutely, I meant "guarantee" as in "as part of the contract between a safe library and its clients", not as in "UB".

[Gankra]

I don't think catch_unwind is notably different from a destructor in this case?

[RalfJung]

I don't think so either.

[RalfJung]

As one example for what this guarantee enables, I claim (if I made no mistake) that this is a safe abstraction (idea and name shamelessly stolen from this crate):

#![no_std]
use core::mem;

/// Temporarily move the `T` out of `x`, and pass it through `f`.
/// If `f` returns, write the result back into `x`.
/// If `f` panics, write `fallback` into `x` (or abort if `fallback` is `None`).
pub fn take_mut<T>(x: &mut T, f: impl FnOnce(T) -> T, fallback: Option<T>) {
    // A drop guard to implement the fallback behavior.
    struct Guard<T> { ptr: *mut T, fallback: Option<T> }
    impl<T> Drop for Guard<T> {
        fn drop(&mut self) {
            // This will double-panic and hence abort if there is no fallback.
            let fallback = self.fallback.take().unwrap();
            unsafe { self.ptr.write(fallback); }
        }
    }
    
    let ptr = x as *mut T;
    let guard = Guard { ptr, fallback };
    // Do the thing!
    unsafe { ptr.write(f(ptr.read())); }
    // If we got here, there was no panic. Discharge the guard.
    mem::forget(guard);
}

[RalfJung]

@gankro pointed out that (if I am reading this correctly) BTreeMap iterators binary_heap also rely on drop running for safety (as opposed to just relying on it to avoid leaks, such as Vec::drain).

hashbrown's clear also uses a scope guard, but there it seems you'd "just" end up with a partially cleared HashMap. Though if unsafe code relies on clearing to work properly even in the case of panics, that would still be catastrophic.

[Gankra]

no its for binary_heap’s heapify operation (in case cmp panics)

[RalfJung]

oops sorry, fixed.