Is deadlock possible in this simple scenario?

Question

Please see the following code:

std::mutex mutex;
std::condition_variable cv;
std::atomic<bool> terminate;

// Worker thread routine
void work() {
    while( !terminate ) {
        {
            std::unique_lock<std::mutex> lg{ mutex };
            cv.wait(lg);

            // Do something
        }
        // Do something
    }
}

// This function is called from the main thread
void terminate_worker() {
    terminate = true;
    cv.notify_all();
    worker_thread.join();
}

Is the following scenario can happen?

Worker thread is waiting for signals.
The main thread called terminate_worker();
- The main thread set the atomic variable terminate to true, and then signaled to the worker thread.
- Worker thread now wakes up, do its job and load from terminate. At this step, the change to terminate made by the main thread is not yet seen, so the worker thread decides to wait for another signal.
Now deadlock occurs...

I wonder this is ever possible. As I understood, std::atomic only guarantees no race condition, but memory order is a different thing. Questions:

Is this possible?
If this is not possible, is this possible if terminate is not an atomic variable but is simply bool? Or atomicity has nothing to do with this?
If this is possible, what should I do?

Thank you.

[`std::memory_order`](http://en.cppreference.com/w/cpp/atomic/memory_order) — melak47, Dec 27 '16 at 08:06
@melak47 Please provide more detail. I know this is about `std::memory_order`. But I have no idea how `std::condition_variable` acts with memory order restrictions. — Junekey Jeon, Dec 27 '16 at 08:19
Related: http://stackoverflow.com/questions/8819095/concurrency-atomic-and-volatile-in-c11-memory-model — Leon, Dec 27 '16 at 08:35
@n.m. which is already done. OP asks about ordering/synchronization between atomic store (with `memor_order_seq_cst`) and `condition_variable::notify_all`. — Zeta, Dec 27 '16 at 08:56
"What should I do" use the condition variable correctly. The condition should be protected by a mutex, atomicity is not enough. — n. 'pronouns' m., Dec 27 '16 at 09:00
@Zeta yes just noticed this. Need more coffee in the morning. — n. 'pronouns' m., Dec 27 '16 at 09:01
Read the discussion here http://stackoverflow.com/questions/17101922/do-i-have-to-acquire-lock-before-calling-condition-variable-notify-one. — n. 'pronouns' m., Dec 27 '16 at 10:29
This code works as intended when`terminate` is not atomic. Making it atomic is redundant and wasteful. — Pete Becker, Dec 27 '16 at 17:14

MikeMB · Accepted Answer · 2016-12-27T14:56:26.280

I don't believe, what you describe is possible, as cv.notify_all() afaik (please correct me if I'm wrong) synchronizes with wait(), so when the worker thread awakes, it will see the change to terminate.

However:

A deadlock can happen the following way:

Worker thread (WT) determines that the terminate flag is still false.
The main thread (MT) sets the terminate flag and calls cv.notify_all().
As no one is curently waiting for the condition variable that notification gets "lost/ignored".
MT calls join and blocks.
WT goes to sleep ( cv.wait()) and blocks too.

Solution:

While you don't have to hold a lock while you call cv.notify, you

have to hold a lock, while you are modifying terminate (even if it is an atomic)
have to make sure, that the check for the condition and the actual call to wait happen while you are holding the same lock.

This is why there is a form of wait that performs this check just before it sends the thread to sleep.

A corrected code (with minimal changes) could look like this:

// Worker thread routine
void work() {
    while( !terminate ) {
        {
            std::unique_lock<std::mutex> lg{ mutex };
            if (!terminate) {
                cv.wait(lg);
            }

            // Do something
        }
        // Do something
    }
}

// This function is called from the main thread
void terminate_worker() {
    {
        std::lock_guard<std::mutex> lg(mutex);
        terminate = true;
    }
    cv.notify_all();
    worker_thread.join();
}

In any case, OP should check after wait what's the state of terminate variable. In provided code we cannot distinguash why condition variable was notified. it can be either terminete request or normal synchronization with another thread. — paweldac, Dec 27 '16 at 09:16
@paweldac: Yes, he probably should (without knowing the business logic it is hard to tell), but what does that have to do with the question whether a deadlock can occure or not? — MikeMB, Dec 27 '16 at 09:20
Thank you very much. I found that in the link [http://en.cppreference.com/w/cpp/thread/condition_variable] it is described that "Even if the shared variable is atomic, it must be modified under the mutex in order to correctly publish the modification to the waiting thread." Is it about what you just described? Or is a different story? And also, is it meaningless to make `terminate` atomic? that is, I can use just `bool`, isn't it? — Junekey Jeon, Dec 28 '16 at 00:38

Is deadlock possible in this simple scenario?

1 Answers1