Search code examples
c++multithreadinglambdapredicatecondition-variable

Proper way to use predicate in conditional variable


I want to know if I need to reset the predicate boolean variable inside the scope of the locked mutex. Right now, I have a std::unique_lock with a lambda function as the predicate parameter - The lambda function returns a boolean flag. Does this mean I need to set the boolean flag back to false inside the scope of the lock guard?

It seems to work a lot faster if I don't reset the boolean flag, but I'm not sure if this is the safe way to approach this.

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

std::condition_variable _conditional;
bool processed = false;
std::mutex _mutex;

void Worker() {
    while (true) {
        //Doing other things ..
        //
        {
            //mutating shared data
            std::unique_lock<std::mutex> _lock(_mutex);
            _conditional.wait(_lock, [] { return processed; });
            processed = false? //do I set this boolean back to false or just leave as is?
            data++;
        }
    }
}

void Reader() {
   //just prints out the changed data
    while (true) {
        std::cout << data << std::endl;
        processed = true;
        _conditional.notify_one();
    }

}

int main(){

   std::thread t1(Worked);
   std::thread t2(Reader);

   t1.join();
   t2.join();

}


Solution

  • Firstly, the Reader never acquires a lock to synchronize its access to the shared data (in this case the processed boolean and the data variable, whatever it is) with the Worker. As such, the Reader can modify processed while the Worker is reading from it, and the Reader can read from data while the Worker is writing to it; these are both race conditions. They can be fixed by having the Reader also lock the mutex before modifying processed or reading from data. The rest of this answer assumes that this correction is made.

    Secondly, whether or not processed should be reset back to false is dependent on what you want the application to do, so it's necessary to understand the consequences.

    If it is never reset to false, then the Worker will never again wait on the condition variable (though it will continuously reacquire the mutex and check the value of processed, despite the fact that it is guaranteed to be true after the first wait terminates), and it will simply keep incrementing data. Even if you correctly synchronize access to the shared data like I mentioned, this still might not do what you want it to do. It's very possible that the Worker could acquire the mutex several times in a row before the Reader, and thus data can be incremented multiple times in-between prints, and data can be printed multiple times in-between increments (there is no order guarantee for printing and incrementing, in such a case)

    If you reset processed back to false after each wait within the Worker, then you can guarantee that data will be printed at least once in-between each increment, since it would be unable to increment data until the Reader has notified it (which requires at least one print first). However, it may still be printed multiple times in-between each increment, because there is still no mechanism forcing the Reader to wait on the Worker.

    If you provide another mechanism allowing the Reader to also wait on the Worker, then you can theoretically guarantee that each print happens exactly once in-between each increment (alternating). But once you've gone this far, the entire process is run serially, so there's really no point in using multiple threads anymore.

    Notice that each of these approaches all have entirely different semantics. The one you should choose depends on what you want your application to do.