How to fix a C++ thread deadlock example

Question

I have a contrived a C++11 thread deadlock. This is achieved using two separate functions with a pool of multiple threads. How could this example be fixed in order to avoid the deadlock? I think the solution has something to do with consistent ordering of locking procedures.

#include <thread>
#include <mutex>
#include <iostream>

std::mutex kettle;
std::mutex tap;

#define THREAD_POOL 8

void kettle_tap(){

    std::cout << "Locking kettle in " << std::this_thread::get_id() << std::endl;
    // Lock the mutex kettle by creating and using lock_guard kettle_lock.
    std::lock_guard<std::mutex> kettle_lock(kettle);
    std::cout << "Locked kettle in " << std::this_thread::get_id() << std::endl;

    std::cout << "Locking tap in " << std::this_thread::get_id() << std::endl;
    // Lock the mutex tap by creating and using lock_guard tap_lock.
    std::lock_guard<std::mutex> tap_lock(tap);
    std::cout << "Locked tap in " << std::this_thread::get_id() << std::endl;

    std::cout << "Filling kettle in " << std::this_thread::get_id() << std::endl;
}

void tap_kettle(){

    std::cout << "Locking tap in " << std::this_thread::get_id() << std::endl;
    // Lock the mutex tap by creating and using lock_guard tap_lock.
    std::lock_guard<std::mutex> tap_lock(tap);
    std::cout << "Locked tap in " << std::this_thread::get_id() << std::endl;

    std::cout << "Locking kettle in " << std::this_thread::get_id() << std::endl;
    // Lock the mutex kettle by creating and using lock_guard kettle_lock.
    std::lock_guard<std::mutex> kettle_lock(kettle);
    std::cout << "Locked kettle in " << std::this_thread::get_id() << std::endl;

    std::cout << "Filling kettle in " << std::this_thread::get_id() << std::endl;

}

int main(){

    std::thread pool[THREAD_POOL];

    for (int t = 0; t < THREAD_POOL; t += 2){
        pool[t]   = std::thread(kettle_tap);
        pool[t+1] = std::thread(tap_kettle);
    }

    for (int t = 0; t < THREAD_POOL; ++t){
        pool[t].join();
    }

    std::cout << "Threads are all joined" << std::endl;

    return 0;

}

Answer 1

std::lock(Mutexes...)

In your case both kettle_tap() and tap_kettle() should begin with:

std::lock(tap, kettle);

but the order of mutex-arguments does not matter, and so can differ across both functions.

Lock multiple mutexes

Locks all the objects passed as arguments, blocking the calling thread if necessary.

The function locks the objects using an unspecified sequence of calls to their members lock, try_lock and unlock that ensures that all arguments are locked on return (without producing any deadlocks).

If the function cannot lock all objects (such as because one of its internal calls threw an exception), the function first unlocks all objects it successfully locked (if any) before failing.

Later on, if you want to transfer the lock's ownership to std::lock_guard:

std::lock(tap, kettle);
std::lock_guard<std::mutex> kettle_lock(kettle, std::adopt_lock);
std::lock_guard<std::mutex> tap_lock(tap, std::adopt_lock);

Answer 2

You are right. Deadlock can be prevented by avoiding circular wait. In your example, to avoid deadlocks, move kettle_lock above tap_lock in the tap_kettle method. This way you get a partial ordering.