Bug 46752 - Differentiate waitForSyncReply from waitForReply
Summary: Differentiate waitForSyncReply from waitForReply
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-28 13:50 PDT by Anders Carlsson
Modified: 2010-09-29 04:20 PDT (History)
1 user (show)

See Also:


Attachments
Patch (7.21 KB, patch)
2010-09-28 13:55 PDT, Anders Carlsson
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2010-09-28 13:50:58 PDT
Differentiate waitForReply from waitForSyncreply
Comment 1 Anders Carlsson 2010-09-28 13:55:26 PDT
Created attachment 69097 [details]
Patch
Comment 2 Anders Carlsson 2010-09-28 14:00:42 PDT
Committed r68559: <http://trac.webkit.org/changeset/68559>
Comment 3 Adam Roben (:aroben) 2010-09-29 04:20:39 PDT
Comment on attachment 69097 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69097&action=review

> WebKit2/Platform/CoreIPC/Connection.cpp:208
> +    while (!timedOut) {
> +        MutexLocker locker(m_waitForSyncReplyMutex);
> +
> +        // First, check if there is a sync reply at the top of the stack.
> +        ASSERT(!m_pendingSyncReplies.isEmpty());
> +            
> +        PendingSyncReply& pendingSyncReply = m_pendingSyncReplies.last();
> +        ASSERT(pendingSyncReply.syncRequestID == syncRequestID);
> +            
> +        // We found the sync reply, return it.
> +        if (pendingSyncReply.didReceiveReply)
> +            return pendingSyncReply.releaseReplyDecoder();
> +
> +        // We didn't find a sync reply yet, keep waiting.
> +        timedOut = !m_waitForSyncReplyCondition.timedWait(m_waitForSyncReplyMutex, absoluteTime);
> +    }

This code is a little strange with respect to the mutex and condition variable. When you enter timedWait, m_waitForSyncReplyMutex is unlocked. When you exit timedWait, m_waitForSyncReplyMutex is locked again. But then you unlock/lock it right away when you start the loop over.

I think a better pattern would be to move the locker local variable outside of the while loop. That way you'll lock the mutex before entering the loop, unlock it each time you wait, relock it each time you finish waiting, and unlock it when the function exits.