|Summary:||DOM Storage's condition variable needs to handle spurious wakeups|
|Product:||WebKit||Reporter:||Jeremy Orlow <jorlow>|
|Component:||New Bugs||Assignee:||Jeremy Orlow <jorlow>|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
Description Jeremy Orlow 2009-10-29 12:58:28 PDT
DOM Storage's condition variable needs to handle spurious wakeups.
Comment 2 Darin Adler 2009-10-29 13:13:50 PDT
Comment on attachment 42127 [details] Patch v1 Is there any way to test this? r=me
Comment 3 Darin Fisher (:fishd, Google) 2009-10-29 13:15:25 PDT
Comment on attachment 42127 [details] Patch v1 > Index: WebCore/ChangeLog ... > + Reviewed by NOBODY (OOPS!). > + > + DOM Storage's condition variable needs to handle spurious wakeups > + https://bugs.webkit.org/show_bug.cgi?id=30920 > + > + DOM Storage's condition variable needs to handle spurious wakeups. ^^^ repeating the summary? > Index: WebCore/storage/LocalStorageThread.cpp > =================================================================== > --- WebCore/storage/LocalStorageThread.cpp (revision 50240) > +++ WebCore/storage/LocalStorageThread.cpp (working copy) > @@ -112,7 +112,8 @@ void LocalStorageThread::terminate() > > m_queue.append(LocalStorageTask::createTerminate(this)); > > - m_terminateCondition.wait(m_terminateLock); > + while (!m_queue.isKilled()) > + m_terminateCondition.wait(m_terminateLock); nit: I think a more explicit 'bool m_terminated' member would be a bit nicer, purely so that one doesn't have to read the MessageQueue class to understand what is going on here. Also, I don't think that createTerminate needs to happen while m_terminateLock is held.
Comment 4 Jeremy Orlow 2009-10-29 13:17:38 PDT
(In reply to comment #2) > (From update of attachment 42127 [details]) > Is there any way to test this? I really wish there was, but I haven't been able to find a way to reliably reproduce the problem--inside or out of a layout test. Maybe if we had some sort of unit test framework I could induce it. Some day, I'd like to investigate how we could incorporate one into WebKit, but I'm pretty sure the answer today is no. :-(