Deadlock simulation using std::mutex

Question

I have following example:

template <typename T>
class container
{
public:
    std::mutex _lock;
    std::set<T> _elements;

    void add(T element)
    {
        _elements.insert(element);
    }

    void remove(T element)
    {
        _elements.erase(element);
    }
};

void exchange(container<int>& cont1, container<int>& cont2, int value)
    {
        cont1._lock.lock();
        std::this_thread::sleep_for(std::chrono::seconds(1));

        cont2._lock.lock();

        cont1.remove(value);
        cont2.add(value);

        cont1._lock.unlock();
        cont2._lock.unlock();
    }

    int main() 
    {
        container<int> cont1, cont2;

        cont1.add(1);
        cont2.add(2);

        std::thread t1(exchange, std::ref(cont1), std::ref(cont2), 1);
        std::thread t2(exchange, std::ref(cont2), std::ref(cont1), 2);

        t1.join();
        t2.join();

        return 0;
    }

In this case I'm expiriencing a deadlock. But when I use std::lock_guard instead of manually locking and unlocking mutextes I have no deadlock. Why?

void exchange(container<int>& cont1, container<int>& cont2, int value)
{
    std::lock_guard<std::mutex>(cont1._lock);
    std::this_thread::sleep_for(std::chrono::seconds(1));

    std::lock_guard<std::mutex>(cont2._lock);

    cont1.remove(value);
    cont2.add(value);
}

Answer 1

Your two code snippets are not comparable. The second snippet locks and immediately unlocks each mutex as the temporary lock_guard object is destroyed at the semicolon:

std::lock_guard<std::mutex>(cont1._lock);  // temporary object

The correct way to use lock guards is to make scoped variables of them:

{
    std::lock_guard<std::mutex> lock(my_mutex);

    // critical section here

}   // end of critical section, "lock" is destroyed, calling mutex.unlock()

(Note that there is another common error that's similar but different:

std::mutex mu;
// ...
std::lock_guard(mu);

This declares a variable named mu (just like int(n);). However, this code is ill-formed because std::lock_guard does not have a default constructor. But it would compile with, say, std::unique_lock, and it also would not end up locking anything.)

Now to address the real problem: How do you lock multiple mutexes at once in consistent order? It may not be feasible to agree on a single lock order across an entire codebase, or even across a future user's codebase, or even in local cases as your example shows. In such cases, use the std::lock algorithm:

std::mutex mu1;
std::mutex mu2;

void f()
{
    std::lock(mu1, mu2);

    // order below does not matter
    std::lock_guard<std::mutex> lock1(mu1, std::adopt_lock);        
    std::lock_guard<std::mutex> lock2(mu2, std::adopt_lock);
}

In C++17 there is a new variadic lock guard template called scoped_lock:

void f_17()
{
    std::scoped_lock lock(mu1, mu2);

    // ...
}

The constructor of scoped_lock uses the same algorithm as std::lock, so the two can be used compatibly.