5
\$\begingroup\$

I want to implement the following case

Input - integer number n

Start 3 threads. The first thread prints 'a', the second prints 'b', the third prints 'c', then the first thread prints 'a' and so on

Example: Input - 6, Output - "abcabc"

I want to use condition variable to synchronise threads

My code

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

void print(
    std::mutex& mutex, 
    std::condition_variable& to_wait, 
    std::condition_variable& to_notify, 
    int count,
    char ch)
{
    std::unique_lock lock(mutex);
    for (int i = 0; i < count; ++i)
    {
        to_wait.wait(lock);
        std::cout << ch;
        to_notify.notify_all();
    }
};


int main()
{
    const int count = 10;

    std::condition_variable var_a;
    std::condition_variable var_b;
    std::condition_variable var_c;
    std::mutex mutex;

    auto thread_a = std::thread(&print, std::ref(mutex), std::ref(var_a), std::ref(var_b), count, 'a');
    auto thread_b = std::thread(&print, std::ref(mutex), std::ref(var_b), std::ref(var_c), count, 'b');
    auto thread_c = std::thread(&print, std::ref(mutex), std::ref(var_c), std::ref(var_a), count, 'c');

    var_a.notify_one();

    thread_a.join();
    thread_b.join();
    thread_c.join();

    return 0;
}

Сould you check code for race condition and deadlocks?

\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

This looks quite good, there are some details you have overlooked. One is that wait() can return spuriously. This means you cannot trust that the condition variable was really notified. The only safe way is to use some other variable(s) to signal whose turn it is, and check that in a loop, or to use wait() with a predicate.

Another is that you have no guarantee that the threads will have started in the order you specified, or that they even will have started before the main thread executes var_a.notify_one(). Since a wait() will only wait for notifications after the waiting has started, it could thus be that the first notification is lost, and the program will not have written anything. This is solved by the same solution as mentioned above.

Also, notify_all() is unnecessary in your code, you can use notify_one() instead. It doesn't really matter in your program, but just make it a habit whenever you really only need any one thread to wake up.

Some alternatives to using three condition variables:

  • Use just one condition variable, and have some variable protected by the mutex to keep track of the next thread to run. When woken up, threads check that to see if it's their turn, else they wait again.
  • Use three mutexes, one for each thread, they all start out locked. Whenever a thread has printed its character, it unlocks the mutex of the next thread.
  • Since C++20: use three std::barriers or std::binary_semaphores.
  • Since C++20: use a single std::atomic<int> to keep track of whose turn it is, as from C++20 you can use wait() and notify_all() directly on atomic variables.
\$\endgroup\$
1
  • \$\begingroup\$ The code isn't working in 100% cases. Ыometimes I have invalid output \$\endgroup\$
    – mascai
    Aug 25, 2023 at 13:26

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.