9

I have bumped into this deadlock scenario while working with Mutex

The struct that contains a field of Mutex type is as follows:

struct MyStruct {
    inner_map: Arc<Mutex<HashMap<i32, Vec<i32>>>>,
}

I have accessed this inner map via lock on Mutex:

impl MyStruct {
    fn test_lock(&self, key: &i32) {
        let my_option = self.inner_map.lock().unwrap().remove(key);
        if let Some(my_vec) = my_option {
            for _inner_val in my_vec {
                self.inner_map.lock().unwrap();
                println!("Passed test_lock1");
            }
        }
    }
}

This is working fine without a deadlock since I removed the value from HashMap and get the ownership out of the HashMap


Very similar function to test_lock with only difference instead of declaring removed value to my_option variable used it on the fly if let call and it is causing deadlock in this case:

impl MyStruct{
    // Why this function goes to deadlock since remove gets the ownership of the data?
    fn test_lock2(&self, key: &i32) {
        if let Some(my_vec) = self.inner_map.lock().unwrap().remove(key) {
            for _inner_val in my_vec {
                self.inner_map.lock().unwrap();
                println!("Passed test_lock2");
            }
        }
    }
}

What is the main reason why declaring a variable changes that kind of behavior?

Playground

nnnmmm
  • 7,964
  • 4
  • 22
  • 41
Akiner Alkan
  • 6,145
  • 3
  • 32
  • 68
  • I suspect you do something inherently unsafe even in your first non deadlocking code. It's hard to tell without the real code but are you sure you shouldn't just have one lock for both the test and the operations you do when the condition is true ? – Denys Séguret Jun 20 '19 at 13:13

1 Answers1

9

The lock is released when the LockResult goes out of scope.

Now we have to go deeper in scope rules regarding this temporary value.

The scope of a temporary value is the enclosing statement.

In the first snippet, it means the lock goes out of scope before entering the if/let construct. There's no deadlock.

But the scope of the temporary value in the if let condition is the whole if/let construct:

the lifetime of temporary values is typically

  • the innermost enclosing statement; the tail expression of a block is considered part of the statement that encloses the block, or
  • the condition expression or the loop conditional expression if the temporary is created in the condition expression of an if or in the loop conditional expression of a while expression.

When a temporary value expression is being created that is assigned into a let declaration, however, the temporary is created with the lifetime of the enclosing block instead, as using the enclosing let declaration would be a guaranteed error (since a pointer to the temporary would be stored into a variable, but the temporary would be freed before the variable could be used)

In the second snippet, the lock's scope thus covers the whole if/let construct.

This explains why the first lock is still active when you try to lock again in the loop.

Denys Séguret
  • 372,613
  • 87
  • 782
  • 758
  • The way the spec was written before, with explicit mention of if/let, was much clearer. Now this feels a little ambiguous... – Denys Séguret Jun 20 '19 at 10:05
  • 1
    I just got bitten by this behavior with a RefCell. `if let Some(_) = cell.borrow().compute_stuff() { cell.borrow_mut().boom(); }`: no borrow, so I thought it would be fine, but the guard spanning the entire `if` forced me to compute first and then match :) – Matthieu M. Jun 20 '19 at 12:43