Help figuring out where drop is called in this case

Why does calling get_or_init cause a deadlock?

#[derive(Clone)]
struct A(i32);
impl A {fn incremented(&self) -> i32 {self.0 + 1}}

struct M {field: RwLock<Option<A>>}

impl M {
    fn get_or_init(&self) -> i32 {
        //let my_a = self.field.read().unwrap().clone();
        //my_a.map_or_else(
        { self.field.read().unwrap().clone() }.map_or_else(
            || {
                let mut field = self.field.write().unwrap();
                *field = Some(A(10));
                A(10).incremented()
            },
            |a| a.incremented(),
        )
    }
}

(Playground)

get_or_init causes a deadlock. I'm surprised, as I would have expected the field.read() read guard to drop within the { } block.

If I use the commented code, I don't get the deadlock. So presumably, it's a misunderstanding on my part as to when stuff is dropped. So why is the read guard not dropped earlier?

Figuring out the drop location based on the reference is quite hard: https://doc.rust-lang.org/reference/destructors.html

My understanding is that the read guard is a "temporary" and looking the section on temporaries, it appears that it only gets dropped at the end of the entire function scope. Is that what it is?

If that's the case, I need to give a 2nd look to a lot of code I reviewed earlier this month, because that assumption was backed in a lot of the code I looked at :see_no_evil_monkey:

you are encountering the second rule here :

temporaries of tail expressions of blocks are dropped immediately after the tail expression is evaluated.
this makes it so there is no deadlock only in edition 2024 and later.

you should use edition 2024 on your project, and you will have no deadlock.
you can also specify edition 2024 on the playground with the ... button on the top left

wow! thank you. I'm saved then, we already use edition 2024 at work.

I was confused by "tail expression of block". In this context, it simply means "the last expression in the block" right?

yes

This is not related to your question about drop order, but something you should keep in mind whenever you use a Mutex or RwLock is that another thread might be accessing it. In particular, your code might end up overwriting something other than a None that got written in the time between read() and write().

Possible solutions:

  • If the value is initialized only once and not mutated again, then you should use std::sync::OnceLock instead — it supports this access pattern efficiently, and also allows borrowing the value without any guard objects.

  • Lock only once using write(), and, if this is the only way the lock is used, use a Mutex instead of a RwLock. This may be more efficient even though it does not permit simultaneous accesses at all.

  • During the write() lock, check if the value is still None before overwriting it.

  • Use parking_lot::RwLock and upgradable_read(). This allows upgrading the read lock to a write lock without unlocking first.