Is this a bug?(unlock is too late...)


#1
use std::thread;
use std::sync::{Arc, Mutex};
use std::sync::mpsc;

fn main() {
    let data = Arc::new(Mutex::new(0));
    for _ in 0..10 {
        let local_data = data.clone();
        thread::spawn(move || {
            println!("start............");
            let mut th_data = local_data.lock().unwrap();
            *th_data += 1;
            println!("Unlocking the data here is best.");
            thread::sleep_ms(5000); //do some job cost a lot of time.
            println!("Unlocking here is too late.");
        });
    }
    thread::sleep_ms(50000);
}

the shared data is too late to unlock…
except only cost a little more than 5s, but actually, cost more than 50s…


#2

No, that is not a bug. Drops happen at the end of the scope or when you give up ownership.

You can avoid this by explicitly dropping the lock:

let mut th_data = local_data.lock().unwrap();
*th_data += 1;
drop(th_data);

Or alternatively, move the operation into a (possibly inline-able) function:

fn do_my_stuff(data: &Mutex<i32>) {
    let mut th_data = local_data.lock().unwrap();
    *th_data += 1;
}

#3

A MutexGuard (return value of lock) only unlocks the Mutex in its destructor, which only runs when the value goes out of scope. In this case, that is the }); line.

You can explicitly move a value out of scope and hence runs its destructor using the drop function:

thread::spawn(move || {
    println!("start............");

    let mut th_data = local_data.lock().unwrap();
    *th_data += 1;

    drop(th_data); // this line

    println!("Unlocking the data here is best.");
    thread::sleep_ms(5000); //do some job cost a lot of time.
    println!("Unlocking here is too late.");
});

Alternatively, you can introduce a smaller scope, so that the th_data variable is only live for a shorter period:

thread::spawn(move || {
    println!("start............");

    {
        let mut th_data = local_data.lock().unwrap();
        *th_data += 1;
    } // th_data goes out of scope here

    println!("Unlocking the data here is best.");
    thread::sleep_ms(5000); //do some job cost a lot of time.
    println!("Unlocking here is too late.");
});

#5

I’m new to rust…thanks.
I have thought rust’s dropping happens at the end of use.


#6

Which do you think is the preferred style? Explicitly dropping the guard or using an inner scope? I’ve used both and I couldn’t say which I prefer, though the scope solution makes it clear when the guard is active.


#7

I have a weirdly irrational distaste for mem::drop.

Well, it’s not totally irrational. It doesn’t relinquish borrows like a scope does.


#8

Yeah, that’s one thing that still bugs me about dropck semantics. You’d think that moving a borrowed value into a function would make its lifetime end after that function returns. Is it just too much work to determine that?


#9

The difference is that mem::drop gives you a (limited) ability to decide lifetime at runtime.

use std::mem;
struct Noisy;
impl Drop for Noisy {
    fn drop(&mut self) {
        println!("dropped");
    }
}

fn demo(flag: bool) {
    let z = Noisy;
    if flag {
        mem::drop(z);
    }
    println!("may be before or after drop");
}

fn main() {
    demo(false);
    demo(true);
}

If you don’t need that, use an inner scope – it’s fully static and makes the borrow checker happier.


#10

Non-lexical lifetimes AKA SEME regions would solve this. Reportedly “falls out” of MIR, so hopefully not long!


#11

In my opinion drop or even introducing inner scopes is a good sign that you are embedding a concern in a function that should be somewhere else. (a separate function)