CODEWITHSUNDEEP
c++multithreadingc++11stdcondition-variable
bobbydev
bobbydev

Reputation: 525

Use of C++ condition variables with atomic predicate but no mutex

I have two threads. One thread acts as a timer thread which at regular intervals of time needs to send a notification to another thread. I intend to use C++ condition variables. (There is a good article on how to use C++ condition variables along with its traps and pitfalls in the following link)

I have the following constraints/conditions :-

  1. The notifying thread need not lock on to a mutex
  2. The notified (or the receiver) thread does some useful section but there is no critical section
  3. The receiver thread is allowed to miss a notification if and only if it is doing useful work
  4. There should be no spurious wakeups.

Using the above link as a guideline I put together the following piece of code

// conditionVariableAtomic.cpp

#include <atomic>
#include <condition_variable>
#include <iostream>
#include <thread>
#include <iostream>       // std::cout, std::endl
#include <thread>         // std::this_thread::sleep_for
#include <chrono>         // std::chrono::seconds


std::mutex mutex_;
std::condition_variable condVar;

std::atomic<bool> dataReady{false};

void waitingForWork(){
    int i = 0;
    while (i++ < 10)
    {
        std::cout << "Waiting " << std::endl;
        {
            std::unique_lock<std::mutex> lck(mutex_);
            condVar.wait(lck, []{ return dataReady.load(); });   // (1)
            dataReady = false;
        }
        std::cout << "Running " << std::endl;
        // Do useful work but no critical section.
    }
}

void setDataReady(){
    int i = 0;
    while (i++ < 10)
    {
        std::this_thread::sleep_for (std::chrono::seconds(1));
        dataReady = true;
        std::cout << "Data prepared" << std::endl;
        condVar.notify_one();
    }
}

int main(){
    
  std::cout << std::endl;

  std::thread t1(waitingForWork);
  std::thread t2(setDataReady);

  t1.join();
  t2.join();
  
  std::cout << std::endl;
  
}

I use an atomic predicate to avoid spurious wakeups, but don't use a lock_guard in the notifying thread. My question is:

  1. does the above piece of code satisfy the constraints/conditions listed above?
  2. I understand that the receiver thread cannot avoid a mutex, hence the need to use std::unique_lock<std::mutex> lck(mutex_); in the receiver. I have however limited the scope of std::unique_lock<std::mutex> lck(mutex_); i.e. put the following section of code 
    std::unique_lock<std::mutex> lck(mutex_);
condVar.wait(lck, []{ return dataReady.load(); });   // (1)
dataReady = false;
    inside a scope block aka { .... } so that the mutex is unlocked as soon as the wait condition is over (the receiver then does some useful work but since there is no critical section, it does not need to hold on to the mutex for the entire while loop). Could there still be consequences/side effects of this limited scoping in this context ? Or does the unique_lock<std::mutex> need to be locked for the entire while loop?

Upvotes: 1

Views: 1488

Answers (1)

Alan Birtles
Alan Birtles

Reputation: 36379

Your code has a race condition. Between checking the value of dataReady in your wait predicate and actually starting the wait, the other thread can set dataReady and call notify_one. In your example this isn't critical as you'll just miss one notify and wake up a second later on the next one.

Another race condition is that you can set dataReady to true in one thread, set dataReady back to false in the other thread and then call notify_one in the first thread, again this will cause the wait to block for longer than you intended.

You should hold the mutex in both threads when setting dataReady and using the condition variable to avoid these races.

You could avoid the second race condition by using an atomic counter instead of a boolean, incrementing it on one thread then decrementing on the other and in the predicate checking if it is non-zero.

Upvotes: 3

Related Questions