Two std::unique_lock used on same mutex causes deadlock ?

Question

I found this code on code review stack exchange which implements a producer-consumer problem. I am posting a section of code here.

In the given code, let's consider a scenario when producer produces a value by calling void add(int num), it acquires lock on mutex mu and buffer.size()==size_ this makes the producer go on wait queue due to the conditional variable cond.

At the same moment, a context switch takes place and consumer calls function int remove() to consume value , it tries to acquire the lock on mutex mu , however the lock has already been acquired previously by the producer so it fails and never consumes the value, hence causing a deadlock.

Where am I going wrong here ? Because the code seems to work properly when I run it, debugging it didn't help me.

Thanks

void add(int num) {
        while (true) {
            std::unique_lock<std::mutex> locker(mu);
            cond.wait(locker, [this](){return buffer_.size() < size_;});
            buffer_.push_back(num);
            locker.unlock();
            cond.notify_all();
            return;
        }
    }
    int remove() {
        while (true)
        {
            std::unique_lock<std::mutex> locker(mu);
            cond.wait(locker, [this](){return buffer_.size() > 0;});
            int back = buffer_.back();
            buffer_.pop_back(); 
            locker.unlock();
            cond.notify_all();
            return back;
        }
    }

Activism

wait should release lock till condition is met and reacquire it before exiting wait – Ludeman

while (true) {..; return;}... why using loop ? – Dubrovnik

Answer 1

OutOfBound's answer is good, but a bit more detail on exactly what is "atomic" is useful.

The wait operation on a condition variable has a precondition and a postcondition that the passed in mutex is locked by the caller. The wait operation unlocks the mutex internally and does so in a way that is guaranteed not to miss any notify or notify_all operations from other threads that happen as a result of unlocking the mutex. Inside wait the unlock of the mutex and entering a state waiting for notifies are atomic with respect to each other. This avoids sleep/wakeup races.

The conditional critical section form tests the predicate internally. It still depends on notifies being done correctly however.

In some sense, one can think of wait as doing this:

while (!predicate()) {
    mutex.unlock();
    /* sleep for a short time or spin */
    mutex.lock();
}

The condition variable with notifies allows the commented line in the middle to be efficient. Which gives:

while (!predicate()) {
    atomic { /* This is the key part. */
        mutex.unlock();
        sleep_until_notified();
    }
    mutex.lock();
}

Lublin

Answer 2

The idea for std::condition_variable::wait(lock, predicate), is that you you wait until the predicate is met and have the lock on mutex afterwards. To do this atomically (which is important most of the time) you have to lock the mutex first, then the wait will release it and lock it for checking the predicate. If it is met the mutex stays locked and the execution continues. If not, the mutex will be released again.

Barbarian