5

std::lock is used to prevent deadlock, right? However in my testing, it still caused deadlock. Could you check my test code to see if I used it incorrectly?

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(2));
    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(2));
    std::unique_lock<std::mutex> lock2(m1, std::defer_lock);
    printf("func2 lock m1\n");
    std::lock(m2, m1);
    printf("func2 std lock\n");
}



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

The output is : func1 lock m1 func2 lock m2 func2 lock m1 func1 lock m2 func2 std lock

Then console hung...

tillman
  • 139
  • 2
  • 9
  • 7
    I think you're confused. Locks aren't used to *prevent* deadlocks, they're what *cause* them. You need to use them properly. – Mark Ransom Aug 22 '17 at 15:21
  • 1
    [`std::lock` Locks the given Lockable objects lock1, lock2, ..., lockn using a deadlock avoidance algorithm to avoid deadlock.](http://en.cppreference.com/w/cpp/thread/lock). – nwp Aug 22 '17 at 15:27
  • 1
    I'm not sure why your code doesn't work, but typically you would pass the (deferred) unique_locks to `std::lock`, not the underlying mutex. – Kerrek SB Aug 22 '17 at 15:36
  • Careful design prevents deadlock. Locks are one of the tools that you can use once you've done the design work. Without careful design the results won't be good. A fool with a tool is still a fool. – Pete Becker Aug 22 '17 at 17:29
  • @PeteBecker: That may be true, but even careful design may incorporate the need for dynamic deadlock avoidance, and that's not an entirely trivial subject. – Kerrek SB Aug 23 '17 at 12:59

1 Answers1

7

I think what you're trying to do doesn't work: You cannot silently modify the mutex underneath the unique lock. According to the specification, the "deferred" constructor makes the lock guard "not owning", and you cannot change that:

unique_lock(mutex_type& m, defer_lock_t) noexcept;

Effects: Constructs an object of type unique_lock.

Postconditions: pm == addressof(m) and owns == false.

The only way to modify the exposition-only owns variable is by acting on the unique lock. The unique lock does not magically inspect the state of the held mutex.

The correct code should pass the unique lock to the std::lock algorithm:

std::lock(lock1, lock2);
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • There were two ways to use std::lock. One is lock_guard, another is unique_lock. I didn't notice they caused std::lock different parameters. http://en.cppreference.com/w/cpp/thread/lock – tillman Aug 22 '17 at 15:48
  • @tillman: You can also use a unique_lock to adopt. Basically, you can do whatever you like to create a scoped guard to do the unlocking, or you could unlock the mutexes manually. Lots of options. "Adopting" a lock obviously makes the guard "own" the lockable. – Kerrek SB Aug 22 '17 at 15:50
  • I appreciate your explanation that is very helpful – tillman Aug 22 '17 at 15:59