1 
#include <thread>
#include <mutex>
#include <condition_variable>
#include <iostream>
#include <chrono>
using namespace std;
using namespace std::chrono_literals;

condition_variable cv;
mutex mu;

void thread1()
{
    mu.lock();
    unique_lock lck(mu);
    cv.wait(lck);
    cout << 1;
}

void thread2()
{
    this_thread::sleep_for(1s);
    mu.lock();
    cv.notify_all();
    cout << 2;    
}

int main()
{
    thread t1(thread1);
    thread t2(thread2);
    this_thread::sleep_for(2s);
}

This code was expected to show number 1 and 2, but shows nothing. If a condition variable has waited with a mutex, then the mutex is unlocked, right?

Improve this question
6
  • The code is not correct. It has a condition race. thread2 may lock mutex first and raise notify to nowhere. sleep(1) is absolutely wrongly used as a synchronization point. sleep(2) is absolutely wrong too. You get a program terminate. Use t1.join() and t2.join(). See the proper usage example: std::condition_variable Commented Aug 16, 2023 at 6:18
  • The code as posted is calling mu.lock() without any corresponding mu.unlock(), which leads to problems. Get rid of your explicit calls to mu.lock(), and add a unique_lock<mutex> lck(mu); line to thread2(). Also add t1.join(); t2.join() to the end of main() to guarantee that your program won't exit before the threads do. Commented Aug 16, 2023 at 6:19
  • 1
    Note a condition_variable is not a variable it is a signal. So you always need an extra variable to test once the condition variable is signalled. You should also ALWAYS wait with a predicate (which will test your variable), this is to avoid problems with spurious wakeups. Read more here : modernescpp.com/index.php/…. Commented Aug 16, 2023 at 6:21
  • Also never lock mutexes manually, use [std::unique_lock(en.cppreference.com/w/cpp/thread/unique_lock) to lock and unlock your mutex for you., This avoids deadlocks when your function exits somewhere halfway due to a return statement or an exception being thrown. Commented Aug 16, 2023 at 6:23
  • 1
    My answers here might be relevant: (1) stackoverflow.com/questions/71882767/…, (2) stackoverflow.com/questions/72260569/…. Commented Aug 16, 2023 at 6:37

2 Answers 2

Reset to default
3

Several issues with the code:

  • thread2 is locking the mutex, but not unlocking it.

  • thread1 is locking it twice. Once with the explicit mu.lock and again with the unique_lock constructor. This is undefined behavior - usually an automatic deadlock so the developer knows they have coded it incorrectly. The constructor of unique_lock that only takes a mutex as a parameter will automatically call mu.lock for you. And the destructor will call mu.unlock.

  • condition variables need some sort of "condition" to check to guard against spurious wakeup. In other words, when thread 1 wakes up, it can't assume that thread 2 was the one that woke it up. It's not a good pattern to assume so. So while having the mutex locked, the code needs to check for "has the thing I'm waiting for actually happened". And if not, then it needs to go back to waiting. I've updated your code below to simulate thread 2 setting a global variable and having thread 1 wait for it to change.

  • No need to sleep(2) in main. Just wait for both threads to be done to decide it's ok to exit main.

Closer to what you want:


condition_variable cv;
mutex mu;
bool has_value_been_set = false;
int value = 0;

void thread1()
{
    unique_lock<mutex>lck(mu);
    while (has_value_been_set == false)
    {
        cv.wait(lck);
    }
    
    cout << "thread1: the value set by the other thread is: " << value << "\n";

}
void thread2()
{
    sleep(1);
    {
        unique_lock<mutex>lck(mu);
        value = 42;
        has_value_been_set = true;
        cout << "thread2: setting the value: " << value << "\n";
    }

    cout << "thread2: notifying other threads: " << value << "\n";
    cv.notify_all();

    
}
int main()
{
    thread t1(thread1);
    thread t2(thread2);


    // wait for both threads to be done
    t1.join();
    t2.join();

}
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

Like this :

#include <chrono>
#include <iostream>
#include <thread>
#include <condition_variable>
#include <mutex>
#include <future>

using namespace std::chrono_literals;

std::condition_variable cv;
std::mutex mu;
int value{ 1 }; // (one of) the values associated with the condition_variable

void thread1()
{
    std::unique_lock<std::mutex> lock{mu};

    // always wait with a predicate, in this case I use a lambda expression
    // this prevents the wait from falling through
    // if the predicate is not true
    // sometimes wait wakes up even if the condition_variable has not been notified (they sometimes wakeup due to OS limitations)
    cv.wait(lock, [&] {return value == 1; });
    std::cout << "1\n";

    value = 2;
    // notify all blocked thread so their waits can check their predicates
    cv.notify_all();
}

void thread2()
{
    std::unique_lock<std::mutex> lock{mu};
    cv.wait(lock, [&] {return value == 2; });
    std::cout << "2\n";
}

int main()
{
    // in C++20 you can also use std::jthread (which will join in destructor)
    // or if you use std::thread don't forget to call join 
    auto future1 = std::async(std::launch::async, thread1); // destructor of future will join at program shutdown
    auto future2 = std::async(std::launch::async, thread2);
    
    std::cout << "waiting for 2" << "\n";
    future2.get();
    std::cout << "done";
    return 0;
}
Improve this answer

2 Comments

While this code block may answer the question, it would be best if you could provide a little explanation for why it does so.
@DanielLangr Added a some more comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.