3
votes

I'm trying to understand how to better use condition variables, and I have the following code.

Behavior.

The expected behavior of the code is that:

  1. Each thread prints "thread n waiting"
  2. The program waits until the user presses enter
  3. When the user presses enter, notify_one is called once for each thread
  4. All the threads print "thread n ready.", and exit

The observed behavior of the code is that:

  1. Each thread prints "thread n waiting" (Expected)
  2. The program waits until the user presses enter (Expected)
  3. When the user presses enter, notify_one is called once for each thread (Expected)
  4. One of the threads prints "thread n ready", but then the code hangs. (???)

Question.

Why does the code hang? And how can I have multiple threads wait on the same condition variable?

Code

#include <condition_variable>
#include <iostream>
#include <string>
#include <vector>
#include <thread>

int main() {

    using namespace std::literals::string_literals; 
    auto m = std::mutex(); 

    auto lock = std::unique_lock(m);

    auto cv = std::condition_variable(); 

    auto wait_then_print =[&](int id) {
        return [&, id]() {
            auto id_str = std::to_string(id); 
            std::cout << ("thread " + id_str + " waiting.\n"); 
            
            cv.wait(lock); 
            // If I add this line in, the code gives me a system error:
            // lock.unlock();

            std::cout << ("thread " + id_str + " ready.\n"); 
        };
    };

    auto threads = std::vector<std::thread>(16); 

    int counter = 0; 
    for(auto& t : threads)
        t = std::thread(wait_then_print(counter++));

    std::cout << "Press enter to continue.\n"; 

    std::getchar(); 

    for(int i = 0; i < counter; i++) {
        cv.notify_one();
        std::cout << "Notified one.\n";
    }

    for(auto& t : threads)
        t.join(); 
}

Output

thread 1 waiting.
thread 0 waiting.
thread 2 waiting.
thread 3 waiting.
thread 4 waiting.
thread 5 waiting.
thread 6 waiting.
thread 7 waiting.
thread 8 waiting.
thread 9 waiting.
thread 11 waiting.
thread 10 waiting.
thread 12 waiting.
thread 13 waiting.
thread 14 waiting.
thread 15 waiting.
Press enter to continue.

Notified one.
Notified one.
thread 1 ready.
Notified one.
Notified one.
Notified one.
Notified one.
Notified one.
Notified one.
Notified one.
Notified one.
Notified one.
Notified one.
Notified one.
Notified one.
Notified one.
Notified one.
1
You're calling cv.wait(lock) when you don't hold the lock -- undefined behavior, but generally will acquire the lock first. Since you never then release the lock, only the first thread gets to print its message and all the rest wait for a lock that is never released...Chris Dodd

1 Answers

4
votes

This is undefined behavior.

In order to wait on a condition variable, the condition variable must be waited on by the same exact thread that originally locked the mutex. You cannot lock the mutex in one execution thread, and then wait on the condition variable in another thread.

auto lock = std::unique_lock(m);

This lock is obtained in the main execution thread. Afterwards, the main execution thread creates all these multiple execution threads. Each one of these execution threads executes the following:

       cv.wait(lock)

The mutex lock was not acquired by the execution thread that calls wait() here, therefore this is undefined behavior.

A more closer look at what you are attempting to do here suggests that you will likely get your intended results if you simply move

auto lock = std::unique_lock(m);

inside the lambda that gets executed by each new execution thread.

You also need to simply use notify_all() instead of calling notify_one() multiple times, due to various race conditions. Remember that wait() automatically unlocks the mutex and waits on the condition variable, and wait() returns only after the thread successfully relocked the mutex after being notified by the condition variable.