There is an inherent race when waiting on the condition variable and the timeout fires. The waitFor() can timeout just before getting a notification from another thread. AutomaticThread::start() uses a condition variable to wait for work from another thread. The other thread waits for the machine thread to respond to the notification, but instead it goes to sleep since it doesn't see the notification.
Created attachment 297335 [details] Patch
Comment on attachment 297335 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=297335&action=review > Source/WTF/wtf/AutomaticThread.cpp:50 > + m_condition.notifyOne(); This is sort of wrong since it makes notifyOne mean notifyTwo. Programs that use notifyOne often rely on the bounded nature of the wake-up. I think that one solution is to simply move this call to the bottom of the method: only m_condition.notifyOne() if we did not find any threads to wake up ourselves. > Source/WTF/wtf/AutomaticThread.cpp:212 > + if (!awokenByNotify && m_isWaiting) { > + m_isWaiting = false; Do we still need awokenByNotify?
(In reply to comment #2) > Comment on attachment 297335 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=297335&action=review > > > Source/WTF/wtf/AutomaticThread.cpp:50 > > + m_condition.notifyOne(); > > This is sort of wrong since it makes notifyOne mean notifyTwo. > > Programs that use notifyOne often rely on the bounded nature of the wake-up. > I think that one solution is to simply move this call to the bottom of the > method: only m_condition.notifyOne() if we did not find any threads to wake > up ourselves. That makes sense. I'll make that change. > > Source/WTF/wtf/AutomaticThread.cpp:212 > > + if (!awokenByNotify && m_isWaiting) { > > + m_isWaiting = false; > > Do we still need awokenByNotify? No. I'll remove it.
Created attachment 297339 [details] Addressed Phil's comments and fixed m_condition ASSERTs
Committed r209938: <http://trac.webkit.org/changeset/209938>
<rdar://problem/29229439>