CODEWITHSUNDEEP
multithreadingc++11mutexcondition-variable
markt1964
markt1964

Reputation: 2836

How to synchronize this, nicely?

Given the following C++11 code fragment:

#include <condition_variable>
#include <mutex>
std::mutex block;
long count;
std::condition_variable cv;

void await()
{
    std::unique_lock<std::mutex> lk(block);
    if (count > 0)
        cv.wait(lk);
}

void countDown()
{
    std::lock_guard<std::mutex> lk(block);
    if (count > 0)
    {
        count--;
        if (count==0) cv.notify_all();
    }
}

If it is not clear what I am trying to accomplish, I am wanting calls to await to pause the calling thread while count is greater than 0, and if it has already been reduced to zero, then it should not pause at all. Other threads may call countDown() which will wake all threads that had previously called await.

The above code seems to work in all cases that I've tried, but I have this nagging doubt about it, because it seems to me like there is a possibility for unexpected behavior if the thread calling await() just happens to get preempted immediately after its condition test has been evaluated and just before the thread is actually suspended by the cv.wait() call, and if the countDown function is getting called at this time, and the count equals 0, then it would issue a notify to the condition variable, IF it were actually already waiting on it... but the thread calling await hasn't hit the cv.wait() call yet, so when the thread calling await resumes, it stops at the cv.wait() call and waits indefinitely.

I actually haven't seen this happen yet in practice, but I would like to harden the code against the eventuality.

Upvotes: 0

Views: 83

Answers (1)

Howard Hinnant
Howard Hinnant

Reputation: 219448

It is good that you are thinking about these possibilities. But in this case your code is correct and safe.

If await gets preempted immediately after its condition test has been evaluated and just before the thread is actually suspended by the cv.wait() call, and if the countDown function is getting called at this time, the latter thread will block while trying to obtain the block mutex until await actually calls cv.wait(lk).

The call to cv.wait(lk) implicitly releases the lock on block, and thus now another thread can obtain the lock on block in countDown(). And as long as a thread holds the lock on block in countDown() (even after cv.notify_all() is called), the await thread can not return from cv.wait(). The await thread implicitly blocks on trying to re-lock block during the return from cv.wait().

Update

I did make a rookie mistake while reviewing your code though <blush>.

cv.wait(lk) may return spuriously. That is, it may return even though it hasn't been notified. To guard against this you should place your wait under a while loop, instead of under an if:

void await()
{
    std::unique_lock<std::mutex> lk(block);
    while (count > 0)
        cv.wait(lk);
}

Now if the wait returns spuriously, it re-checks the condition, and if still not satisfied, waits again.

Upvotes: 4

Related Questions