Threads do their job one after another but not "simultaneously"

     let mut v = Arc::new(Mutex::new(String::new()));
    let v2 = Arc::clone(&v);
   let h =  thread::spawn(move||{
        for _ in 0..5{
          //  let x = Arc::clone(v);
           let mut n =  v2.lock().unwrap();
           *n+="th";
           thread::sleep(Duration::from_millis(2));
           
        }     
    });
    for _ in 0..5{
        let mut n = v.lock().unwrap();
        *n+="mn";
        thread::sleep(Duration::from_millis(1))   
    }
    h.join();
    println!("{:?}",v.lock().unwrap());

I would like to get something similar to "mnthmnthmnthmnthmn", but I get "mnmnmnmnmnththththth" instead? What should I do to improve this peace of code, so that the h thread doesn't run after the main thread, but the run in "parallel"?

Note that your ns are each a MutexGuard, which holds the lock until it is dropped -- and this includes your sleep time. If you explicitly drop(n), the execution is better mixed.

8 Likes

Your violating the golden rule of multithreading, as I like to call it.

As things currently stand, you lock the mutex at the start of each loop iteration and that mutex lock is held until the iteration ends. However, you don't give any other thread a chance to execute because the loop runs again, and so no other thread gets the chance to actually do anything useful.

You also have a sleep in the loop; however, this is still within the locked region -- called a critical section -- so nothing can be done.

The golden rule of multithreading is that you should only lock mutexes (called "acquiring" a lock) when you actually are going to do something that requires you to acquire a lock. As soon as you are done doing whatever it is that you needed to do, unlock the mutex (this is called "releasing" the lock). You have two options to solve this problem:

  1. You can put the lock acquisition and work that needs to be done in its own block within the loop
  2. You can explicitly call unlock on the mutex.

Option 1 looks like this:

           {
               let mut n =  v2.lock().unwrap();
                *n+="th";
            }
            thread::sleep(Duration::from_millis(2));

Option 2 looks like this:

           let mut n = v2.lock().unwrap();
           *n+="th";
            v2.unlock();
           thread::sleep(Duration::from_millis(2));

The first option is more ergonomic than the second.

4 Likes

Not sure how it is expected to work. To unlock the mutex, one have to drop the MutexGuard - how can method on the mutex itself achieve this?

2 Likes

I actually meant to pass in the guard that lock() generated -- unlock() requires a guard so you'd call it like Mutex::unlock(guard). Dropping the guard is one way of unlocking it, but you can explicitly call Mutex::unlock to force it to happen if you don't wanna use drop.

Also note that Mutex::unlock has not been stabilized yet (#81872).

2 Likes

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.