| Summary: | unlocking mutex before calling notify_one/all on condition variable triggers an error | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Yuval Lifshitz <yuvalif> |
| Component: | helgrind | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED NOT A BUG | ||
| Severity: | normal | CC: | bart.vanassche+kde, ivosh |
| Priority: | NOR | ||
| Version: | 3.10.0 | ||
| Target Milestone: | --- | ||
| Platform: | Debian stable | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: | file causing the error | ||
|
Description
Yuval Lifshitz
2017-01-01 11:20:04 UTC
POSIX [1] also states that the lock does not need to be held: "The pthread_cond_broadcast() or pthread_cond_signal() functions may be called by a thread whether or not it currently owns the mutex that threads calling pthread_cond_wait() or pthread_cond_timedwait() have associated with the condition variable during their waits; however, if predictable scheduling behavior is required, then that mutex shall be locked by the thread calling pthread_cond_broadcast() or pthread_cond_signal()." The above applies to mutexes that have priorities associated with them, that is, either ceiling mutexes (PTHREAD_PRIO_PROTECT) or priority-inheritance mutexes (PTHREAD_PRIO_INHERIT). Normal mutexes do not contain any priority scheduling; therefore there is usually a (tiny) performance benefit to call mutex_unlock() before calling cond_signal(). [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_cond_signal.html Why is this behavior considered to be a bug? Not holding the associated mutex around pthread_cond_signal() may lead to missed wakeups. It is easy to see in the example on http://en.cppreference.com/w/cpp/thread/condition_variable that a wakeup may be missed. Inserting a call to sleep() after std::unique_lock<std::mutex> lk(m) and before cv.wait(lk, []{return processed;}) will cause the wakeup not to be noticed. what about the following case:
we change:
cv.wait(lk, []{return ready;});
to (which should be similar):
while (!ready) cv.wait(lk);
in this case, as long as there is only one thread waiting on the condition the program would always be correct - even if we add sleep before the waiting.
the waiter cannot be sleeping while "ready" is modified, because it is mutex protected. this mean that either "ready" was modified before the waiter takes its mutex, and then it does not even wait. or it is waiting and then it is getting notified.
note that even after the code is modified helgrind still gives the same error
I do not agree that a loop like "while (!ready) cv.wait(lk);" would be a correct way to wait for cv to be signaled. If a sleep() statement would be inserted before or inside that loop the wake-up could still be missed. didn't say that wake-up is not missed. but the program would still perform correctly, since the protected variables "ready" and "processed" are either being checked inside the mutex or being waited on using the condition variable. downside for signaling when not locked is explained here: http://www.domaigne.com/blog/computing/condvars-signal-with-mutex-locked-or-not/ but this is not the case in the sample from cppreference |