Multiple mutable references in scope

Hi there,

today I've opened this issue:
https://github.com/rust-lang/rust/issues/54749

Can someone explain to me, what happens there? I always thought, having multiple mutable references to the same object in scope at the same time is illegal, but rust does not seem to care in this case (but in many other cases it does!) and instead only complains when the first reference is used after the second comes into scope.

And why does the code eventually fail when I implement the Drop trait for the struct Ref?

Best regards!

AFAICS, your code is only accepted with #![feature(nll)], and even then only when there's no possibility for the Ref.0 to be accessed.

Nope, it's accepted fine without enabling any features (on nightly). And it's even possible to access the Ref.0 field.

Only because you're likely using the 2018 Edition, which already has NLL enabled by default.

The more interesting case is:

This likely falls out from the fact that r1's lifetime is extended to the end of the block (scope) where it's dropped, but this now overlaps with r2's borrow. I'm not sure if this is a limitation of NLL or merely the fact that "implicit" Drops occur at a specified place (i.e. scope end) and thus here to stay. Anyway, the workaround would be to put r1 into its own scope or explicitly call drop(r1) right after it's printed (and before r2 comes into the picture).

This is now NLL is defined; Drop still extends to the end of the scope. This is because we can't risk changing it; it has a high chance of breaking unsafe code in the wild. This feature was attempted separately for NLL and was called "eager drop" if you want to poke about on the internet to learn more.

2 Likes

Right, makes sense. To that end, one could use #[may_dangle] on the drop impl and indicate that we don't access fields of Ref, and therefore the borrows should be allowed to overlap. And indeed, that makes borrowck happy.

1 Like

So @Socke, does this discussion make sense? I'm happy to elaborate if it's not clear from these answers!

I wanted to make sure, that only one such mapping can exist at the same time (because memory needs to be flushed which happens in the drop() of the mapping before another mapping can be created)

And why does the code eventually fail when I implement the Drop trait for the struct Ref?

I'm confused. Doesn't the second quote mean you have nothing to worry about with regards to the first quote?

@ExpHP I'm not totally convinced this is the case. I forgot to include the output of the program:

First reference
Second reference
Done
Dropping!
Dropping!

So from what I have learned from the discussion, the first Drop should occour before the second reference is created because r1's lifetime isn't extended to the end of the function.

@steveklabnik: It would be totally great if you can just elaborate a little bit more, I'm still a little bit confused.

Right, and this requires that your Ref<'a> impls Drop. Then the compiler will prevent r2 from coming into existence unless r1 was dropped.

Alright, but why does the compiler decide to not care about it when I don't impl Drop for my Ref<'a>?

In fact in my real code I would end up implementing no-op Drops just to get the compiler convinced to really prevent having multiple Mappings (which are similar to the Refs) in place.

So that's the NLL feature in play - instead of tracking borrows purely lexically, it tracks them more precisely. Once r1 is effectively dead (i.e. no longer used), whatever it borrowed is essentially canceled out. For "normal" Rust, this is a huge improvement in usability/ergonomics.

A Drop impl means drop is (implicitly, in this case) called at precisely scope end, and since drop has access to self, the borrow checker does not allow r2 until r1 is dropped (or else you'd have multiple mutable borrows at the same time).

You mentioned that your Drop flushes the memory behind the mapping, so why are you saying you'd implement no-op Drop? If r1 is effectively dead, and has no Drop, then what unsoundness can occur if r2 comes into existence?

Okay, so NLL will land in Rust 1.31. So we can talk about two cases: before 1.31, and after.

Before 1.31, with the old lifetime rules. Everything goes out of scope at the end of the lexical scope it was defined in. Furthermore, things are dropped in the reverse order they're declared:

fn main() {
    let mut v = Val;                            <- first, v
    println!("First reference");
    let r1 = v.reference();                     <- second, r1
    println!("Second reference");
    let r2 = v.reference();                     <- third, r2
    println!("Done");

    //println!("{:?}", r1.0);
} <- everything goes out of scope here, and is dropped in reverse order: r2, r1, v

Therefore, r1 and r2 both exist for the same length of time r2 exists, so you have two mutable references.

After 1.31, with "non-lexical liftimes". The rules are now "things that implement Drop go out of scope at the end of the lexical scope they're defined in. Things that don't implement Drop go out of scope at the last time they're used.

fn main() {
    let mut v = Val;                            <- first, v
    println!("First reference");
    let r1 = v.reference();                     <- second, r1. v is used here, so it must live this long. r1 is not used again, and so drops immediately.
    println!("Second reference");
    let r2 = v.reference();                     <- third, r2. v is used here, so it must live this long. r2 is not used again, and so drops immediately.
                                                <- additionally, v is never used again, and so is dropped here
    println!("Done");

    //println!("{:?}", r1.0);
} <- we have nothing still in scope

Now, there's two things that change this: uncommenting the println, and the Drop impl. If we uncomment the println:

fn main() {
    let mut v = Val;                            <- first, v
    println!("First reference");
    let r1 = v.reference();                     <- second, r1. v is used here, so it must live this long.
    println!("Second reference");
    let r2 = v.reference();                     <- third, r2. v is used here, so it must live this long. r2 is not used again, and so drops immediately.
    println!("Done");

    println!("{:?}", r1.0);                     <- r1 is used here, so it must live this long
}

Now that you use r1 below, it stays alive longer, and now comes into conflict with r2 being created. Error!

With Drop for Ref:

fn main() {
    let mut v = Val;                            <- first, v
    println!("First reference");
    let r1 = v.reference();                         let r1 = v.reference();                     <- second, r1. v is used here, so it must live this long. r1 implements `Drop`, and so will go out of scope at the end of `main`
    println!("Second reference");
    let r2 = v.reference();                     <- third, r2. v is used here, so it must live this long. r2 implements `Drop`, and so will go out of scope at the end of `main` 
    println!("Done");

    // println!("{:?}", r1.0);
} <- r2 would drop, then r1

This also makes r1 live long enough to come into conflict with r2.

Does that clear it up?

1 Like

Yeah thank you very much, now it's pretty clear to me what the new ruling is.

But why have you decided for the "things that implement Drop go out of scope at the end of the lexical scope" but not for something like "things that implement Drop or that contain something that implements Drop go out of scope at the end of the lexical scope"?

That seems like a pitfall to me where I change the drop semantics of some member-types just by implementing Drop myself.

And when the r1 goes out of scope right after it's no longer used, why is the drop of it's Refed not called immediately? Aren't values dropped immediately once they go out of scope?

The two questions are kind of the same just from a different point of view.

I don’t remember all of the details, but it has to do with existing expectations, thanks to the historical usage. Unsafe code is much harder to write, and easier to mess up, if the drop location changes based on use. Additionally, crates like scopeguard would no longer work. Basically, it would have broken too much existing code. Non-Drop stuff doesn’t really need to rely on when it’s dropped, by definition.

It is true that it creates two classes of rules, but you have the compiler to help you, as you experienced :slight_smile:

Okay, again, thank you very much for your kind help! Now I'm starting to feel better :wink:

I'd like to echo this.

It seems awfully clear to me that you should not have to write no-op drop impls under any conditions, and if adding one to your code fixes a bug or API exploit, the real bug is elsewhere.

Your stdout from the playground example is meaningless. If your Mappings need to flush the buffer then they won't have a no-op Drop.

wait a second, wait a second, wait a second...

Okay. Here's what's weird about your example: You have an object (Refed) whose Drop affects another value, yet has a 'static lifetime. This is the real footgun, and is not something that typically happens in rust. (you would have to be using statics with interior mutability, or maybe thin wrappers around types from FFI... or maybe *shudders* unsafe)

I would suggest changing this so that Refed, wherever created, borrows from Val. Use PhantomData if need be.

use std::marker::PhantomData;

struct Refed<'a>(PhantomData<&'a mut ()>);

impl<'a> Drop for Refed<'a> {
    fn drop(&mut self) {
        println!("Dropping!");
    }
}

struct Ref<'a>(&'a mut Val, Refed<'a>);


#[derive(Debug)]
struct Val;

impl Val {
    fn reference(&mut self) -> Ref<'_> {
        Ref(self, Refed(PhantomData))
    }
}

Now the borrow checker will care about when Ref gets dropped (even without the no-op Drop impl), because it cares about when Refed gets dropped (and that's what actually matters).


This does somewhat bother me. It hints towards a way in which the introduction of NLL could be introducing footguns into formerly safe APIs. (and they'll hardly ever be noticed because hardly anyone writes compilefail tests for their crate because they involve way too much effort!)

Unfortunately the Refed in the real world is a memmap::MmapMut. It doesn't has static lifetime, instead it should not live any longer than the FileMapping (which is similar to Val) I obtained it from via some map method (which is the counterpart to reference in the above minimal example).

So unfortunately I have no control of what "Refed" actually looks like in the real world and instead I'm trying to write a safe wrapper around the unsafe memmap crate.

Suggestions? I could wrap the memmap::MmapMut (which is created in FileMapping::map()) in another struct which also has the PhantomData member and thus the lifetime bound. Would this help?

Rats. Then I think you really do need a no-op Drop impl, at least somewhere, even if it is in a small lifetime-bound wrapper around MmapMut. That sucks.