CODEWITHSUNDEEP
c++multithreadingc++11condition-variable
user3078395
user3078395

Reputation: 11

Why condition_variable in this code blocks?

I'm new to condition variables and I wonder why this piece of code blocks after counter variable is equal to 99? Deleting the for loop and putting "counter += 99" instead makes the code work, does it have anything to do with sleep_for? Thanks for help :)

#include<thread>
#include<condition_variable>
#include<mutex>
#include<chrono>
#include <iostream>
std::condition_variable cv;
std::mutex mtx;
int counter = 0;

void foo() {
    std:: unique_lock<std::mutex>lck{ mtx };
    //counter += 99;
    for (; counter < 100; counter++) {
        std::cout << counter << std::endl;
        std::this_thread::sleep_for(std::chrono::milliseconds(20));
    }
    lck.unlock();
    cv.notify_one();
}

int main() {
    std::thread th(&foo);
    {
        std::unique_lock<std::mutex>lck{ mtx };
        cv.wait(lck, [] {
            return counter == 99;
        });
    }

    std::cout << "!!!!!" << std::endl;
    th.join();
}

Upvotes: 0

Views: 162

Answers (2)

bku_drytt
bku_drytt

Reputation: 3249

#include<thread>
#include<condition_variable>
#include<mutex>
#include<chrono>
#include <iostream>
std::condition_variable cv;
std::mutex mtx;
int counter = 0;

void foo() {
    std:: unique_lock<std::mutex>lck{ mtx };
    //counter += 99;
    for (; counter < 100; counter++) {                                // B
        std::cout << counter << std::endl;
        std::this_thread::sleep_for(std::chrono::milliseconds(20));
    }
    lck.unlock();
    cv.notify_one();                                                  // C
}

At the end of your loop (B), counter has a value of 100.

int main() {
    std::thread th(&foo);
    {
        std::unique_lock<std::mutex>lck{ mtx };
        cv.wait(lck, [] {
            return counter == 99;                                     // A
        });
    }

    std::cout << "!!!!!" << std::endl;
    th.join();
}

Your condition to wake up the thread running main() is that counter has a value of 99 (A).

Because you call unlock() after your loop is finished (C), the value of counter is 100. Your std::condition_variable will only wake up its the waiting thread when the value of counter is 99 (A). Therefore, unless there is a spurious wakeup in the waiting thread when counter has a value of 99 (which would cause the std::condition_variable's condition to evaluate to true), your waiting thread will be stuck waiting forever.

You can fix this by simply changing the std::condition_variable's predicate as follows:

    {
        std::unique_lock<std::mutex>lck{ mtx };
        cv.wait(lck, [] {
            return counter == 100; // CHANGED TO 100 FROM 99.
        });
    }

Or (the exclusive kind) by changing the condition in your for() loop as follows:

for (; counter < 99; counter++) { // `counter < 100` is now `counter < 99`
    std::cout << counter << std::endl;
    std::this_thread::sleep_for(std::chrono::milliseconds(20));
}

Upvotes: 0

Barry
Barry

Reputation: 303357

Let's add some annotation to your code:

void foo() {
    std:: unique_lock<std::mutex>lck{ mtx };
    // counter == 0
    for (; counter < 100; counter++) {
        ...
    }
    // counter == 100
    lck.unlock();
    cv.notify_one(); // <== notify here
}

We're looping until counter == 100, at which point we notify cv. However, we are waiting for counter == 99, which is not true at the point when it gets notified. The only way for wait() to return in your code is for a spurious wakeup to happen right at the last iteration of the loop.

Perhaps you meant to loop while counter < 99.

Upvotes: 1

Related Questions