Condition variable basic example

Question

I am learning condition variables in C++11 and wrote this program based on a sample code.

The goal is to accumulate in a vector the first ten natural integers that are generated by a producer and pushed into the vector by a consumer. However it does not work since, for example on some runs, the vector only contains 1, 7 and 10.

#include <mutex>
#include <condition_variable>
#include<vector>
#include <iostream>
#include <cstdio>

std::mutex mut;
#define MAX     10
int counter;
bool isIncremented = false;
std::vector<int> vec;
std::condition_variable condvar;

void producer() {
    while (counter < MAX) {
        std::lock_guard<std::mutex> lg(mut);
        ++counter;
        isIncremented = true;
        condvar.notify_one();
    }
}

void consumer() {
    while (true) {
        std::unique_lock<std::mutex> ul(mut);
        condvar.wait(ul, [] { return isIncremented; });
        vec.push_back(counter);
        isIncremented = false;
        if (counter >= MAX) {
            break;
        }
    }
}

int main(int argc, char *argv[]) {
    std::thread t1(consumer);
    std::thread t2(producer);
    t2.join();
    t1.join();

    for (auto i : vec) {
        std::cout << i << ", ";
    }
    std::cout << std::endl;
    // Expected output: 1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
    // Example of actual output: 1, 7, 10,

    std::cout << "Press enter to quit";
    getchar();
    return 0;
}

Answer 1

The problem is that you only remember the last number your producer produced. And your producer never waits until the consumer has consumed what it produced. If your producer thread gets to do more than one iteration of its loop before the consumer thread gets to run (which is not unlikely since the loop doesn't do much), the consumer will only see the last number the producer produced and only push that one into the vector…

To solve this problem, either use a second condition variable to make the producer wait for someone to pick up the last result it produced, or use something that can store more than one result between producer and consumer, or a combination thereof…

Note: Notifying a condition variable is not a blocking call. If it were, it would have to ask you to hand over the mutex so it can internally release it or you'd end up in a deadlock. notify_one() will just wake up one of the threads that are waiting on the condition variable and return. The wait call that the woken thread was blocking on will reacquire the mutex before it returns. In your case, it's not unlikely that the consumer thread be woken and then fail to reacquire the mutex and block again right away because your producer thread is still holding on to the mutex when it's calling notify_one(). Thus, as a general rule of thumb, you want to release the mutex associated with a condition variable should you be holding it before you call notify…

Answer 2

A side note, apparently you used the lock_guard<> in producer, but unique_lock in consumer. In the consumer, the unique_lock also doesn't seem to guard the share resource exclusively.

Below is a modified code that uses unique_lock in both producer and consumer, that guard against shared resource counter.

The code adds a sleep in the producer so that the consumer can be notified of the counter change.

Output seems to be as expected.

#include <mutex>
#include <condition_variable>
#include<vector>
#include <iostream>
#include <cstdio>
#include <thread>
#include <chrono>

std::mutex mut;
#define MAX   10
int counter = 0;
bool isIncremented = false;
std::vector<int> vec;
std::condition_variable condvar;

void producer() {
    while (counter < MAX) {
        std::unique_lock<std::mutex> lg(mut);
        ++counter;
        isIncremented = true;
        lg.unlock();

        condvar.notify_one();
        std::this_thread::sleep_for(std::chrono::milliseconds(10));
    }
}

void consumer() {
    while (true) {
        std::unique_lock<std::mutex> ul(mut);
            condvar.wait(ul, [] { return isIncremented; });
        vec.push_back(counter);  
        isIncremented = false;
        if (counter >= MAX) {
            break;
        }
        ul.unlock();
    }
}

int main(int argc, char *argv[]) {
    std::thread t1(consumer);
    std::thread t2(producer);

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

    for (auto i : vec) {
        std::cout << i << ", ";
    }
    std::cout << std::endl;
    return 0;
}

Answer 3

Using @MichaelKenzel suggestions from the answer, here is a working example. std::queue is used in order to store more than one result between producer and consumer.

#include<mutex>
#include<condition_variable>
#include<vector>
#include<iostream>
#include<cstdio>
#include<thread>
#include<queue>

std::mutex mut;
#define MAX     10
int counter;
std::queue<int> data_queue;
std::vector<int> vec;
std::condition_variable condvar;

void producer()
{
    while (counter < MAX)
    {
        ++counter;
        std::lock_guard<std::mutex> lg(mut);
        data_queue.push(counter);
        condvar.notify_one();
    }
}

void consumer()
{
    while (true)
    {
        std::unique_lock<std::mutex> ul(mut);
        condvar.wait(ul, [] { return !data_queue.empty(); });

        int data = data_queue.front();
        data_queue.pop();
        ul.unlock();

        vec.push_back(data);
        if (data >= MAX)
        {
            break;
        }
    }
}

int main(int argc, char *argv[])
{
    std::thread t1(consumer);
    std::thread t2(producer);
    t2.join();
    t1.join();

    for (auto i : vec)
    {
        std::cout << i << ", ";
    }
    std::cout << std::endl;

    return 0;
}