c++multithreadingc++11gcc4.8

C++ thread deadlock on waiting condition


Trying to expand in my two previous questions Move operations for a class with a thread as member variable and Call function inside a lambda passed to a thread

I'm not understanding why the thread doing a wait_for is somtimes not being notified wich results in a deadlock. Cppreference says on condition variables http://en.cppreference.com/w/cpp/thread/condition_variable/notify_one

The notifying thread does not need to hold the lock on the same mutex as the one held by the waiting thread(s); in fact doing so is a pessimization, since the notified thread would immediately block again, waiting for the notifying thread to release the lock.

MCVE, the commented line explains what changes if I hold the lock, but I dont undrestand why:

#include <atomic>
#include <condition_variable>
#include <mutex>
#include <thread>

#include <iostream>

using namespace std;

class worker {
public:
    template <class Fn, class... Args>
    explicit worker(Fn func, Args... args) {
        t = std::thread(
            [&func, this](Args... cargs) -> void {
                std::unique_lock<std::mutex> lock(mtx);
                while (true) {
                    cond.wait(lock, [this]() -> bool { return ready; });
                    if (terminate) {
                        break;
                    }

                    func(cargs...);
                    ready = false;
                }
            },
            std::move(args)...);
    }

    ~worker() {
        terminate = true;
        if (t.joinable()) {
            run_once();
            t.join();
        }
    }

    void run_once() {
        // If i dont hold this mutex the thread is never notified of ready being
        // true.
        std::unique_lock<std::mutex> lock(mtx);
        ready = true;
        cout << "ready run once " << ready << endl;
        cond.notify_all();
    }

    bool done() { return (!ready.load()); }

private:
    std::thread t;
    std::atomic<bool> terminate{false};
    std::atomic<bool> ready{false};
    std::mutex mtx;
    std::condition_variable cond;
};

// main.cpp

void foo() {
    worker t([]() -> void { cout << "Bark" << endl; });
    t.run_once();
    while (!t.done()) {
    }
}

int main() {
    while (true) {
        foo();
    }
    return 0;
}

Solution

  • You need a memory barrier to make sure that the other thread will see the modified "ready" value. "ready" being atomic only ensures that the memory access is ordered so that modifications that happened before the atomic access are actually flushed to main memory. This does not guarantee that the other threads will see that memory, since these threads may have their own cache of the memory. Therefore, to ensure that the other thread sees the "ready" modification, the mutex is required.

    {
      std::unique_lock<std::mutex> lock(mtx);
      ready = true;
    }