Try_lock to have one instance of thread: Why not working?

help
1

I wish to use a mutex to prevent more than one simultaneous instance of a thread.

So I have a Arc<Mutex<()>> and call try_lock on it in a loop that spawns the thread. I expect that the first time around the loop try_lock will return Ok as it is not held. The thread will sleep, holding the lock, for five seconds meaning that the next four calls (each after sleeping one second) will not return Ok so the thread will not be spawned... But that is not what I see so I do not understand how try_lock works. What have I got wrong?

Programme:

use std::sync::{Arc, Mutex};
use std::thread::JoinHandle;
use std::thread::sleep;
use std::thread;
use std::time::Duration;

struct Foo {
    m:Arc<Mutex<()>>,
}
fn main() {
    let mut threads:Vec<JoinHandle<_>> = Vec::new();
    let foo = Foo{m:Arc::new(Mutex::new(()))};
    for _ in 1..10 {
        let m = foo.m.clone();
        let lock = m.try_lock();
        if let Ok(_) = lock {
            threads.push(thread::spawn( move || {
                eprintln!("Start of thread {:?}", thread::current().id());
                sleep(Duration::new(5, 0));
                eprintln!("End of  thread {:?}", thread::current().id());
            }));
        }else{
            eprintln!("Not doing thread");
        }
        sleep(Duration::new(1, 0));
    }
    for jh in threads {
        eprintln!("Join res: {:?}", jh.join());
    }
}

Output:

cargo run
   Compiling test v0.1.0 (file:///home/worik/Documents/2018/Rust/Test)
    Finished dev [unoptimized + debuginfo] target(s) in 0.75s
     Running `target/debug/test`
Start of thread ThreadId(1)
Start of thread ThreadId(2)
Start of thread ThreadId(3)
Start of thread ThreadId(4)
Start of thread ThreadId(5)
End of  thread ThreadId(1)
Start of thread ThreadId(6)
End of  thread ThreadId(2)
Start of thread ThreadId(7)
End of  thread ThreadId(3)
Start of thread ThreadId(8)
End of  thread ThreadId(4)
Start of thread ThreadId(9)
End of  thread ThreadId(5)
Join res: Ok(())
Join res: Ok(())
Join res: Ok(())
Join res: Ok(())
Join res: Ok(())
End of  thread ThreadId(6)
Join res: Ok(())
End of  thread ThreadId(7)
Join res: Ok(())
End of  thread ThreadId(8)
Join res: Ok(())
End of  thread ThreadId(9)
Join res: Ok(())
2

Because you've assigned the lock guard to a variable created inside the loop, the variable is destroyed and the lock guard held in the variable is freed after each iteration of the loop. Nothing else has access to foo, so the lock can never fail to lock.

Try not to use mutex as a lock, but as a wrapper for data to access. Arbitrary Mutex<()> is easy to get wrong. Mutext<the data you need to access> is impossible to free too early.

1 Like
3

No, I do not understand.

lock guard to a variable created inside the loop

The assignment is a clone (?) let m = foo.m.clone();

What should I use? I am locking a section of code not a variable.

4

That's only increase of Arc's refcount. Nothing is copied.

Note that Mutex and MutexGuard are two separate objects. When you lock a Mutex you get its MutexGuard, and destructor of the MutexGuard unlocks the mutex for you.

What should I use? I am locking a section of code not a variable.

In this contrived example there's no other choice, but usually if you rethink your problem, you can find some shared object, resource (e.g. array, file or db handle, channel) that the code under lock works on. If you wrap that shared resource, Rust will ensure for you that a lock is always acquired when needed and never released too early (so code accidentally droppping a MutexGuard won't compile).

for _ in 1..10 {
        let m = foo.m.clone();
        let lock = m.try_lock(); // it's (maybe) locked starting from here
        if let Ok(_) = lock {
           …
        }else{
            …
        }
        sleep(Duration::new(1, 0));
        drop(lock); // inserted by the compiler. Unlocks m.
    }

I'm also not sure about that Ok(_). There's a special case in Rust where let _ = foo instantly destroys foo, so _ in Ok(_) might too, but I haven't checked.

5

This is the core misunderstanding - the spawned thread is not holding the lock at all. The main thread holds the lock temporarily while spawning but that’s it - it’s essentially just a normal loop where nothing contends with the main thread - it’s the only one locking and unlocking.

If you want the background thread to hold the lock, clone m, move it inside the spawn closure, and lock() before going to sleep (make sure to assign the returned guard to a binding). Something like this:

for _ in 1..10 {
        let m = foo.m.clone();
        let lock = m.try_lock();
        if let Ok(g) = lock {
            let clone = foo.m.clone();
            threads.push(thread::spawn(move || {
                eprintln!("Start of thread {:?}", thread::current().id());
                let _g = clone.lock().unwrap();
                sleep(Duration::new(5, 0));
                eprintln!("End of  thread {:?}", thread::current().id());
            }));
        } else {
            println!("Not doing thread");
        }
        sleep(Duration::new(1, 0));
    }