Is std::condition_variable thread-safe?

Question

I have some code like this:

std::queue<myData*> _qDatas;
std::mutex _qDatasMtx;

Generator function

void generator_thread()
{
  std::this_thread::sleep_for(std::chrono::milliseconds(1));
  {
    std::lock_guard<std::mutex> lck(_qDatasMtx);
    _qData.push(new myData);
  }
  //std::lock_guard<std::mutex> lck(cvMtx); //need lock here?
  cv.notify_one();
}

Consumer function

void consumer_thread()
{
  for(;;)
  {
    std::unique_lock lck(_qDatasMtx);
    if(_qDatas.size() > 0)
    {
       delete _qDatas.front();
       _qDatas.pop();
    }
    else
      cv.wait(lck);
  }
}

So if I have dozens of generator threads and one consumer thread, do I need a mutex lock when calling cv.notify_one() in each thread?

And is std::condition_variable thread-safe?

Answer 1

do I need a mutex lock when calling cv.notify_one() in each thread?

No

is std::condition_variable thread-safe?

Yes

---

When calling wait, you pass a locked mutex which is immediately unlocked for use. When calling notify you don't lock it with the same mutex, since what will happen is (detailed on links):

1. Notify
2. Wakeup thread that is waiting
3. Lock mutex for use

From std::condition_variable:

execute notify_one or notify_all on the std::condition_variable (the lock does not need to be held for notification)

and from std::condition_variable::notify_all:

The notifying thread does not need to hold the lock on the same mutex as the one held by the waiting thread(s);

---

With regards to your code snippet:

//std::lock_guard<std::mutex> lck(cvMtx); //need lock here?
cv.notify_one();

No you do not need a lock there.

Answer 2

notify_one can be called from multiple threads without a lock.

However, in order for normal operation to work and no chance of notifications "slipping through", you must hold the lock the condition variable is guarded by between the point where you modify the spurious wakeup guard and/or package the condition variable checks on read, and the point where you notify one.

Your code appears to pass that test. However this version is more clear:

std::unique_lock lck(_qDatasMtx);
for(;;) {
  cv.wait(lck,[&]{ return _qDatas.size()>0; });
  delete _qDatas.front();
  _qDatas.pop();
}

and it reduces spurious unlocking/relocking.