1

Consider the following quote about std::condition_variable from cppreference:

The condition_variable class is a synchronization primitive that can be used to block a thread, or multiple threads at the same time, until another thread both modifies a shared variable (the condition), and notifies the condition_variable.

The thread that intends to modify the variable has to

  1. acquire a std::mutex (typically via std::lock_guard)
  2. perform the modification while the lock is held
  3. execute notify_one or notify_all on the std::condition_variable (the lock does not need to be held for notification)

Even if the shared variable is atomic, it must be modified under the mutex in order to correctly publish the modification to the waiting thread.

An exemplary typical scenario of this approach is as follows:

// shared (e.d., global) variables:
bool proceed = false;
std::mutex m;
std::condition_variable cv;
// thread #1:
{
   std::unique_lock<std::mutex> l(m);
   while (!proceed) cv.wait(l); // or, cv.wait(l, []{ return proceed; });
}
// thread #2:
{
   std::lock_guard<std::mutex> l(m);
   proceed = true;
}
cv.notify_one();

However, @Tim in this question came up with (kind-of an academic) alternative:

std::atomic<bool> proceed {false}; // atomic to avoid data race
std::mutex m;
std::condition_variable cv;
// thread #1:
{
   std::unique_lock<std::mutex> l(m);
   while (!proceed) cv.wait(l);
}
// thread #2:
proceed = true; // not protected with mutex
{ std::lock_guard<std::mutex> l(m); }
cv.notify_one();

It obviously does not meet the requirements of the above cppreference quote since proceed shared variable (atomic in this case) is not modified under the mutex.

The question is if this code is correct. In my opinion, it is, since:

  1. First option is that !proceed in while (!proceed) is evaluated as false. In that case, cv.wait(l); is not invoked at all.

  2. Second option is that !proceed in while (!proceed) is evaluated as true. In that case, cv.notify_one() cannot happen until thread #1 enters cv.wait(l);.

Am I missing something or is cppreference wrong in this regard? (For instance, are some reordering issues involved?)


And, what if proceed = true; would be changed to proceed.store(true, std::memory_order_relaxed);?

Community
  • 1
  • 1
Daniel Langr
  • 22,196
  • 3
  • 50
  • 93
  • `The question is if this code is correct.` Could you define "correct"? What should the code achieve? It should only allow one thread to proceed, right? – KamilCuk Apr 23 '20 at 07:09
  • 1
    @KamilCuk Basically, I ask whether both shown variants have the same effect (and, cannot result in undefined behavior, abnormal program termination, race conditions, deadlocks, etc.). – Daniel Langr Apr 23 '20 at 07:11
  • I think [C++17 atomics and condition_variable deadlock](https://stackoverflow.com/questions/49622713/c17-atomics-and-condition-variable-deadlock/49623578#49623578) contains an example of possible trouble with mixture of atomics and condvars. – user7860670 Apr 23 '20 at 07:13
  • @user7860670 That is a different case, there is no mutex locked between modification of a shared variable and notification in the question you linked. – Daniel Langr Apr 23 '20 at 07:15
  • The key part is that value is not modified under locked mutex. – user7860670 Apr 23 '20 at 07:16

0 Answers0