Std::Lock avoids deadlock but this program gets stuck

Question

All, Referring to the question in std::lock still caused deadlock

I still couldn't figure what is the problem in the below code. Can somebody please explain the problem and how to fix this? Why does it get hung? Pls help.

#include <iostream>
#include <mutex>
#include <thread>
using namespace std;

std::mutex m1;
std::mutex m2;

void func1()
{
    std::unique_lock<std::mutex> lock1(m1, std::defer_lock);
    printf("func1 lock m1\n");
    std::this_thread::sleep_for(std::chrono::seconds(1));
    std::unique_lock<std::mutex> lock2(m2, std::defer_lock);
    printf("func1 lock m2\n");
    std::lock(m1, m2);
    printf("func1 std lock\n");

}

void func2()
{
    std::unique_lock<std::mutex> lock1(m2, std::defer_lock);
    printf("func2 lock m2\n");
    std::this_thread::sleep_for(std::chrono::seconds(1));
    std::unique_lock<std::mutex> lock2(m1, std::defer_lock);
    printf("func2 lock m1\n");
    std::lock(m1, m2);
    printf("func2 std lock\n");
}



int main(int argc,char* argv[])
{
    std::thread th1(func1);
    std::thread th2(func2);
    th1.join();
    th2.join();
    return 0;
}

Output seen: func1 lock m1
func2 lock m2
func1 lock m2
func1 std lock
func2 lock m1
----- Hung here.

Why doesn't func2 proceed even though func1 has released both the mutexes?

Sniffle

Why does it get hung? Because one thread locks both mutexes and does not unlock them. The second thread then waits forever on std::lock call. – Obligation

Answer 1

Instead of :

std::lock(m1, m2);

use :

std::lock(lock1, lock2);

More details (including an example) can be found on the reference page for std::lock.

Why does your code hang ?

When you call std::lock(m1, m2), the two mutexes are locked directly. Neither of the std::unique_locks (lock1 and lock2) are aware of this, and thus they can not unlock the mutexes.

So, when func1 ends both mutexes are still locked, and func2 cannot proceed past the std::lock(m1, m2) line.

Why does the fixed code work ?

When you call std::lock(lock1, lock2), the std::unique_locks (lock1 and lock2) are aware of this - they now own the locks, and are responsible for unlocking them (which happens when they go out of scope).

So, when func1 ends both mutexes are unlocked, and func2 can proceed past the std::lock(lock1, lock2) line. And all is well.

Reversal

In C++17, you may also use std::scoped_lock l(m1,m2); without any need for unique_lock objects. – Obligation

@DanielsaysreinstateMonica : I didn't mention that because of the c++11 tag the op added, but good point nevertheless. – Reversal

You're right, didn't mention C++11 tag. It was just a sidenote. – Obligation

Thanks for pointing out mistake in my answer. Anyway I think that need to lock 2 mutexes the same time may point onto wrong design. – Episcopacy

@Episcopacy : possibly. Sometimes you do need to have multiple resources locked at the same time though. – Reversal

@Note that the question is Can somebody please explain the problem and how to fix this? Why does it get hung? You might want to add some more information. – Obligation

@Episcopacy why? sometimes it is necessary to do so. As long as the mutexes are locked in the same order I dont see the problem – Ionopause

@formerlyknownas_463035818 Using the ordering algorithm can lead to performance problems: howardhinnant.github.io/dining_philosophers.html – Lukewarm

Superb. Got it. Thanks all. The original code was locking the mutexes directly and not via the unique_lock. std::lock(lock1, lock2) fixed the issue. – Sniffle

Answer 2

Unique_lock can't unlock them becuase you have mentioned that These lock are not adopted yet.

std::unique_lock<std::mutex> lock1(m1, std::defer_lock);
std::unique_lock<std::mutex> lock2(m2, std::defer_lock);

So unique lock cant unlock them.

Solution

            void func1()
            {
                std::unique_lock<std::mutex> lock1(m1, std::defer_lock);
                printf("func1 lock m1\n");
                std::this_thread::sleep_for(std::chrono::seconds(1));
                std::unique_lock<std::mutex> lock2(m2, std::defer_lock);
                printf("func1 lock m2\n");
                std::lock(m1, m2);
                printf("func1 std lock\n");

                m1.unlock();
                m2.unlock();

            }

Miyamoto