Closure capturing `Self` lifetime error

use parking_lot::Mutex;

#[derive(Default)]
pub struct MyStruct<'a> {
    closure: Mutex<Option<Box<dyn FnMut() + 'a>>>,
}

impl<'a> MyStruct<'a> {
    fn set_closure(&self, f: Box<dyn FnMut() + 'a>) {
        *self.closure.lock() = Some(f);
    }

    fn call_closure(&self) {
        self.closure.lock().as_mut().map(|closure| closure());
    }

    fn other_fn(&self) {
        println!("Other fn");
    }
}

fn main() {
    let mut resource = "my resource".to_string();

    let my_struct = MyStruct::default();

    my_struct.set_closure(Box::new(|| {
        resource += "+";
        println!("Using {}", &resource);  // this works
        // my_struct.other_fn()  // this doesn't work
    }));

    my_struct.call_closure();
}

Since I could capture resource because it outlives my_struct, why couldn't I capture my_struct even though it outlives itself?

You have to borrow the struct for 'a to create the dyn + 'a and effectively borrow yourself forever. It's a variation on being self-referential, and such a construction is incompatible with having a non-trivial destructor (which all dyn have.

1 Like

I think this makes it more clear:

struct HasDrop<'a, 'b>(&'b MyStruct<'a>);

impl<'a, 'b> Drop for HasDrop<'a, 'b> {
    fn drop(&mut self) {
        println!("MyStruct is still accessible here");
        println!("This happens after MyStruct is dropped");
    }
}

fn main() {
    let my_struct = MyStruct::default();

    let has_drop = HasDrop(&my_struct);

    my_struct.set_closure(Box::new(move || {
        &has_drop;
    }));

    my_struct.call_closure();
}
1 Like

You said "This happens after MyStruct is dropped", but I think this happens before MyStruct is dropped according to Destructors - The Rust Reference and Destructors > Drop scopes - The Rust Reference.

The destructor of a type T consists of:

  1. If T: Drop, calling <T as std::ops::Drop>::drop
  2. Recursively running the destructor of all of its fields.

(I comprehend this as the fields are destructed after drop is called, otherwise it should be illegal for us to access any field of the type when drop runs)

When control flow leaves a drop scope all variables associated to that scope are dropped in reverse order of declaration (for variables) or creation (for temporaries).

(So in your snippet, has_drop is actually dropped before my_struct is dropped.

While we have access to has_drop.0 when <HasDrop as Drop>::drop runs, neither my_struct nor the reference to it (has_drop.0) has been dropped. I don't see the reason why rustc rejects it in this specific case.

Here's a simplified case that doesn't involve anonymous closure types, dyn trait objects or their Drop impls:

use parking_lot::Mutex;

#[derive(Default)]
struct MyMutex<'a> {
    inner: Mutex<Option<&'a MyMutex<'a>>>,
}

// impl<'a> Drop for MyMutex<'a> {  // The code works until you uncomment this `Drop` impl
//     fn drop(&mut self) {}
// }

fn main() {
    let my_mutex = MyMutex::default();
    *my_mutex.inner.lock() = Some(&my_mutex);
}

I just wonder at which point would we have access to an invalid reference if rustc let this snippit compile with the Drop impl uncommented.

has_drop has been moved and therefore doesn't participate in this order. It's now a member of the closure, which is eventually owned by my_struct.

While this technically works when there's only one field, if you instead have this:

pub struct MyStruct<'a> {
    field: Box<i32>,
    closure: Mutex<Option<Box<dyn FnMut() + 'a>>>,
}

field is dropped before closure, which would mean has_drop now has access to a deallocated Box. Unlike Drop impls, there is no field order analysis, so all such cases are rejected.

Yeah you're right, I forgot about move, but what if we don't add move and actually capture a reference (which is intended)?

Or take a look at my last snippet with MyMutex, did you mean that it should technically be legal, but was rejected due to lack of field order analysis?

The conflict between borrowing forever and Drop implementations is that Drop::drop requires an exclusive reference (&mut), and creating one of those is incompatible with being borrowed.

(There's some unsafe, unstable way to tune destructor borrow-checking - may_dangle. I haven't looked to see if it would help your use case or not.)

2 Likes

Maybe we could improve the compiler help message which currently states:

error[E0597]: `my_mutex` does not live long enough
  --> src/main.rs:14:35
   |
13 |     let my_mutex = MyMutex::default();
   |         -------- binding `my_mutex` declared here
14 |     *my_mutex.inner.lock() = Some(&my_mutex);
   |                                   ^^^^^^^^^ borrowed value does not live long enough
15 | }
   | -
   | |
   | `my_mutex` dropped here while still borrowed
   | borrow might be used here, when `my_mutex` is dropped and runs the `Drop` code for type `MyMutex`

though I'm not sure how.

IMO &'a mut Thing<'a> should at least show up in errors, if not be a built-in warning lint. This would be an extension of that, where you have &a Thing<'a> plus a non-trivial destructor.

I checked and may_dangle does work around it (but all the other problems of self-referencial structs would remain).

1 Like