CP.42: Don’t wait without a condition

[Quuxplusone]

trafficstars

More specific than #554 so I figured I'd open a new issue. "CP.42: Don’t wait without a condition" gives this "example, bad":

void thread1() {
    while (true) {
        // do some work ... [for the sake of argument let's say q.push(42)]
        std::unique_lock<std::mutex> lock(mx);
        cv.notify_one();    // wake other thread
    }
}

void thread2() {
    while (true) {
        std::unique_lock<std::mutex> lock(mx);
        cv.wait(lock);    // might block forever
        // do work ... [for the sake of argument let's say q.pop()]
    }
}

Here, if some other thread consumes thread1’s notification, thread2 can wait forever.

That's true, but adding a condition to the wait doesn't actually fix that problem. If the cv.notify_one() can be consumed by "some other thread," then there's really nothing thread2 will ever be able to do about that.

void thread2() {
    while (true) {
        std::unique_lock<std::mutex> lock(mx);
        cv.wait(lock, [](){ return !q.empty(); });    // STILL might block forever
        // do work ... [for the sake of argument let's say q.pop()]
    }
}

I think the original "example, bad" needs to be a complete example; i.e., replace the "do work..." comments with actual code that can be evaluated by the reader. I don't know if it's possible to create an example where

• cv.wait(lock, some_condition) is correct,
• cv.wait(lock) is buggy, and
• cv.wait(lock) isn't just obviously wrong and out of step with the programmer's intent.

Or, maybe the appropriate solution is to replace the words "if some other thread consumes thread1’s notification" with the words "if thread1's notification arrives before thread2 has begun waiting on cv"?

[cubbimew]

Could just weasel-word it out as "thread2 could miss the notification" thread2 is also buggy in that it does work on every spurious wakeup (which aren't theoretical: changing system time fires them on linux)

[Quuxplusone]

@cubbimew: I really think "thread2 could miss the notification" implies the wrong intuition. I want to avoid the implication that a notification could somehow "fail to arrive" thread2, as if thread2 isn't being fully attentive or as if there's some bad guy out there eating the notifications meant for thread2. The problem is specifically that "thread1's notification arrives before thread2 has begun waiting on cv."

[The "bad" version of] thread2 is also buggy in that it does work on every spurious wakeup

Agreed. The current text does actually nod to this problem by saying "...or wake up simply to find that there is no work to do," but it would be a huge improvement for it to actually explain what that means (i.e., define and use the term "spurious wakeup").

FWIW, I frequently use this mailbox/flag/cymbals metaphor for explaining the mutex/cv pattern: https://www.youtube.com/watch?v=jfDRgnxDe7o&t=4m15s (Spurious wakeup is described @6:14.)

[hsutter]

Editors call: Thanks, we should improve this example.