1
votes

If data race is not an issue, can I use std::condition_variable for starting (i.e., signaling) and stopping (i.e, wait) a thread for work?

For example:

std::atomic<bool> quit = false;
std::atomic<bool> work = false;
std::mutex mtx;
std::condition_variable cv;

// if work, then do computation, otherwise wait on work (or quit) to become true
// thread reads: work, quit
void thread1()
{
   while ( !quit )
   {
      // limiting the scope of the mutex
      {
         std::unique_lock<std::mutex> lck(mtx);

         // I want here is to wait on this lambda
         cv.wait(lck, []{ return work || quit; });
      }

      if ( work )
      {
         // work can become false again while working.
         // I want here is to complete the work
         // then wait on the next iteration.
         ComputeWork();
      }
   }
}

// work controller
// thread writes: work, quit
void thread2()
{
   if ( keyPress == '1' )
   {
      // is it OK not to use a mutex here?
      work = false;
   }
   else if ( keyPress == '2' )
   {
      // ... or here?
      work = true;
      cv.notify_all();
   }
   else if ( keyPress == ESC )
   {
      // ... or here?
      quit = true;
      cv.notify_all();
   }
}

Update/Summary: not safe because of 'lost wakeup' scenario that Adam describes. cv.wait(lck, predicate()); can be equivalently written as while(!predicate()){ cv.wait(lck); }.

To see the problem easier: while(!predicate()){ /*lost wakeup can occur here*/ cv.wait(lck); }

Can be fixed by putting any read/writes of predicate variables in the mutex scope:

void thread2()
{
   if ( keyPress == '1' )
   {
      std::unique_lock<std::mutex> lck(mtx);
      work = false;
   }
   else if ( keyPress == '2' )
   {
      std::unique_lock<std::mutex> lck(mtx);
      work = true;
      cv.notify_all();
   }
   else if ( keyPress == ESC )
   {
      std::unique_lock<std::mutex> lck(mtx);
      quit = true;
      cv.notify_all();
   }
}
1
You should be okay because reading the atomic will cause a memory load, and so will acquring the mutex each time the condition variable checks its predicate. I did encounter once, really early on in MSVC's support for condition_variable, a scenario where it didn't properly add a memory fence before reading the atomic that was inside the predicate checked by the condition variable (and the bad codegen eventually went away). - AndyG
The whole point of a condition variable is to wake up a waiting thread atomically, avoiding any kind of sleep. Your update isn't t an ideal solution. You were on the right lines initially, but it is necessary to notify the condition variable after changing a flag. - jignatius

1 Answers

0
votes

No, not safe. The waiting thread can get the mutex, check the predicate, sees nothing to wake up for. Then the signalling thread sets the bool, and signals. Next, the waiting thread blocks on the cv, and never awakens.

You must hold the mutex at some point between triggering the wakeup lambda condition, and notifying the cv, to avoid this.

The "down" case (turning off wakeup) I have not looked at, and it may depend on what behaviour exactly is ok. Without that specified in a formal sense I wouldn't do it either; in general, you should at least attempt sketches of formal proofs of correctness when fiddling with multi threaded code, or your code will be at best accidentally working.

If you can't do that, find someone who can to write that code for you.