6

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?

t.niese
  • 39,256
  • 9
  • 74
  • 101
sudheerbb
  • 63
  • 4
  • 2
    _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. – Daniel Langr Nov 21 '19 at 10:43

1 Answers1

12

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.

Community
  • 1
  • 1
Sander De Dycker
  • 16,053
  • 1
  • 35
  • 40
  • 2
    In C++17, you may also use `std::scoped_lock l(m1,m2);` without any need for `unique_lock` objects. – Daniel Langr Nov 21 '19 at 10:39
  • 1
    @DanielsaysreinstateMonica : I didn't mention that because of the c++11 tag the op added, but good point nevertheless. – Sander De Dycker Nov 21 '19 at 10:40
  • 1
    You're right, didn't mention C++11 tag. It was just a sidenote. – Daniel Langr Nov 21 '19 at 10:41
  • 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. – Alex Nov 21 '19 at 10:41
  • @Alex : possibly. Sometimes you *do* need to have multiple resources locked at the same time though. – Sander De Dycker Nov 21 '19 at 10:42
  • 1
    @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. – Daniel Langr Nov 21 '19 at 10:45
  • 1
    @Alex why? sometimes it is necessary to do so. As long as the mutexes are locked in the same order I dont see the problem – 463035818_is_not_an_ai Nov 21 '19 at 10:46
  • @formerlyknownas_463035818 Using the ordering algorithm can lead to performance problems: http://howardhinnant.github.io/dining_philosophers.html – Howard Hinnant Nov 21 '19 at 13:21
  • 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. – sudheerbb Nov 21 '19 at 13:56