Bug 126161

Summary: Convert ThreadCondition in WebKit2 over to std::condition_variable
Product: WebKit Reporter: Anders Carlsson <andersca>
Component: New BugsAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eflews.bot, gyuyoung.kim, ltilve+ews, philn, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Anders Carlsson
Reported 2013-12-23 07:56:56 PST
Convert ThreadCondition in WebKit2 over to std::condition_variable
Attachments
Patch (9.97 KB, patch)
2013-12-23 08:02 PST, Anders Carlsson 		no flags
DetailsFormatted DiffDiff
Patch (9.97 KB, patch)
2013-12-23 09:06 PST, Anders Carlsson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Anders Carlsson
Comment 1 2013-12-23 08:02:14 PST
Created attachment 219915 [details] Patch
EFL EWS Bot
Comment 2 2013-12-23 08:11:46 PST
Comment on attachment 219915 [details] Patch Attachment 219915 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/45768344
Anders Carlsson
Comment 3 2013-12-23 09:06:40 PST
Created attachment 219916 [details] Patch
EFL EWS Bot
Comment 4 2013-12-23 09:12:08 PST
Comment on attachment 219916 [details] Patch Attachment 219916 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/49998225
WebKit Commit Bot
Comment 5 2013-12-23 10:33:28 PST
Comment on attachment 219916 [details] Patch Clearing flags on attachment: 219916 Committed r161002: <http://trac.webkit.org/changeset/161002>
WebKit Commit Bot
Comment 6 2013-12-23 10:33:30 PST
All reviewed patches have been landed. Closing bug.
Filip Pizlo
Comment 7 2015-08-12 21:37:28 PDT
Comment on attachment 219916 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219916&action=review > Source/WebKit2/Platform/CoreIPC/Connection.cpp:423 > - if (!m_waitForMessageCondition.timedWait(m_waitForMessageMutex, absoluteTime)) { > + // FIXME: It would be better if Connection::waitForMessage took an std::chrono::milliseconds instead of a double. > + std::chrono::milliseconds timeoutInMilliseconds(static_cast<std::chrono::milliseconds::rep>(timeout * 1000)); > + if (m_waitForMessageCondition.wait_for(lock, timeoutInMilliseconds) == std::cv_status::timeout) { This looks like a regression: previously, if there was a spurious return from timedWait() that happened just before the timeout and caused us to loop around, the subsequent timedWait() would wake up after the remainder of the original timeout. After the change, if timedWait() returns spuriously and we loop around, we will wait the full timeout. I.e. spurious returns from wait can cause us to never time out. Was this an intentional change in behavior?
Anders Carlsson
Comment 8 2015-08-13 10:17:49 PDT
> > This looks like a regression: previously, if there was a spurious return > from timedWait() that happened just before the timeout and caused us to loop > around, the subsequent timedWait() would wake up after the remainder of the > original timeout. > > After the change, if timedWait() returns spuriously and we loop around, we > will wait the full timeout. I.e. spurious returns from wait can cause us to > never time out. > > Was this an intentional change in behavior? No, good catch.
Note You need to log in before you can comment on or make changes to this bug.