2011-03-26 Anders Carlsson <andersca@apple.com>

        Reviewed by Sam Weinig.

        Fix a possible deadlock when two synchronous messages are sent at the same time
        https://bugs.webkit.org/show_bug.cgi?id=57155

        Simplify code and fix a possible (although highly improbable) dead lock.

        * Platform/CoreIPC/Connection.cpp:
        Make SyncMessageState atomically ref counted since it can be ref()'ed from the connection queue.
        Get rid of m_waitForSyncReplyCount and add m_didScheduleDispatchMessagesWork.

        (CoreIPC::Connection::SyncMessageState::SyncMessageState):
        Initialize m_didScheduleDispatchMessagesWork to false.

        (CoreIPC::Connection::SyncMessageState::processIncomingMessage):
        if m_didScheduleDispatchMessagesWork is false, schedule a call to dispatchMessageAndResetDidScheduleDispatchMessagesWork
        on the client run loop.

        (CoreIPC::Connection::SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesWork):
        Dispatch messages and set m_didScheduleDispatchMessagesWork back to false.

        (CoreIPC::Connection::sendSyncMessage):
        Remove calls to beginWaitForSyncReply and endWaitForSyncReply.


git-svn-id: svn://svn.chromium.org/blink/trunk@82032 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/WebKit2/ChangeLog

+2011-03-26  Anders Carlsson  <andersca@apple.com>
+
+        Reviewed by Sam Weinig.
+
+        Fix a possible deadlock when two synchronous messages are sent at the same time
+        https://bugs.webkit.org/show_bug.cgi?id=57155
+
+        Simplify code and fix a possible (although highly improbable) dead lock.
+
+        * Platform/CoreIPC/Connection.cpp:
+        Make SyncMessageState atomically ref counted since it can be ref()'ed from the connection queue.
+        Get rid of m_waitForSyncReplyCount and add m_didScheduleDispatchMessagesWork.
+
+        (CoreIPC::Connection::SyncMessageState::SyncMessageState):
+        Initialize m_didScheduleDispatchMessagesWork to false.
+
+        (CoreIPC::Connection::SyncMessageState::processIncomingMessage):
+        if m_didScheduleDispatchMessagesWork is false, schedule a call to dispatchMessageAndResetDidScheduleDispatchMessagesWork
+        on the client run loop.
+
+        (CoreIPC::Connection::SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesWork):
+        Dispatch messages and set m_didScheduleDispatchMessagesWork back to false.
+
+        (CoreIPC::Connection::sendSyncMessage):
+        Remove calls to beginWaitForSyncReply and endWaitForSyncReply.
+
 2011-03-25  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Adele Peterson.

third_party/WebKit/Source/WebKit2/Platform/CoreIPC/Connection.cpp

@@ -36,14 +36,11 @@ using namespace std;
 
 namespace CoreIPC {
 
-class Connection::SyncMessageState : public RefCounted<Connection::SyncMessageState> {
+class Connection::SyncMessageState : public ThreadSafeRefCounted<Connection::SyncMessageState> {
 public:
     static PassRefPtr<SyncMessageState> getOrCreate(RunLoop*);
     ~SyncMessageState();
 
-    void beginWaitForSyncReply();
-    void endWaitForSyncReply();
-
     void wakeUpClientRunLoop()
     {
         m_waitForSyncReplySemaphore.signal();
@@ -76,13 +73,15 @@ private:
         return syncMessageStateMapMutex;
     }
 
+    void dispatchMessageAndResetDidScheduleDispatchMessagesWork();
+
     RunLoop* m_runLoop;
     BinarySemaphore m_waitForSyncReplySemaphore;
 
-    // Protects m_waitForSyncReplyCount and m_messagesToDispatchWhileWaitingForSyncReply.
+    // Protects m_didScheduleDispatchMessagesWork and m_messagesToDispatchWhileWaitingForSyncReply.
     Mutex m_mutex;
 
-    unsigned m_waitForSyncReplyCount;
+    bool m_didScheduleDispatchMessagesWork;
 
     struct ConnectionAndIncomingMessage {
         Connection* connection;
@@ -109,7 +108,7 @@ PassRefPtr<Connection::SyncMessageState> Connection::SyncMessageState::getOrCrea
 
 Connection::SyncMessageState::SyncMessageState(RunLoop* runLoop)
     : m_runLoop(runLoop)
-    , m_waitForSyncReplyCount(0)
+    , m_didScheduleDispatchMessagesWork(false)
 {
 }
 
@@ -121,49 +120,27 @@ Connection::SyncMessageState::~SyncMessageState()
     syncMessageStateMap().remove(m_runLoop);
 }
 
-void Connection::SyncMessageState::beginWaitForSyncReply()
-{
-    ASSERT(RunLoop::current() == m_runLoop);
-
-    MutexLocker locker(m_mutex);
-    m_waitForSyncReplyCount++;
-}
-
-void Connection::SyncMessageState::endWaitForSyncReply()
-{
-    ASSERT(RunLoop::current() == m_runLoop);
-
-    MutexLocker locker(m_mutex);
-    ASSERT(m_waitForSyncReplyCount);
-    --m_waitForSyncReplyCount;
-
-    if (m_waitForSyncReplyCount)
-        return;
-
-    // Dispatch any remaining incoming sync messages.
-    for (size_t i = 0; i < m_messagesToDispatchWhileWaitingForSyncReply.size(); ++i) {
-        ConnectionAndIncomingMessage& connectionAndIncomingMessage = m_messagesToDispatchWhileWaitingForSyncReply[i];
-        connectionAndIncomingMessage.connection->enqueueIncomingMessage(connectionAndIncomingMessage.incomingMessage);
-    }
-
-    m_messagesToDispatchWhileWaitingForSyncReply.clear();
-}
-
 bool Connection::SyncMessageState::processIncomingMessage(Connection* connection, IncomingMessage& incomingMessage)
 {
     MessageID messageID = incomingMessage.messageID();
     if (!messageID.shouldDispatchMessageWhenWaitingForSyncReply())
         return false;
 
-    MutexLocker locker(m_mutex);
-    if (!m_waitForSyncReplyCount)
-        return false;
-
     ConnectionAndIncomingMessage connectionAndIncomingMessage;
     connectionAndIncomingMessage.connection = connection;
     connectionAndIncomingMessage.incomingMessage = incomingMessage;
 
-    m_messagesToDispatchWhileWaitingForSyncReply.append(connectionAndIncomingMessage);
+    {
+        MutexLocker locker(m_mutex);
+        
+        if (!m_didScheduleDispatchMessagesWork) {
+            m_runLoop->scheduleWork(WorkItem::create(this, &SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesWork));
+            m_didScheduleDispatchMessagesWork = true;
+        }
+
+        m_messagesToDispatchWhileWaitingForSyncReply.append(connectionAndIncomingMessage);
+    }
+
     wakeUpClientRunLoop();
 
     return true;
@@ -186,6 +163,17 @@ void Connection::SyncMessageState::dispatchMessages()
     }
 }
 
+void Connection::SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesWork()
+{
+    {
+        MutexLocker locker(m_mutex);
+        ASSERT(m_didScheduleDispatchMessagesWork);
+        m_didScheduleDispatchMessagesWork = false;
+    }
+
+    dispatchMessages();
+}
+
 PassRefPtr<Connection> Connection::createServerConnection(Identifier identifier, Client* client, RunLoop* clientRunLoop)
 {
     return adoptRef(new Connection(identifier, true, client, clientRunLoop));
@@ -357,10 +345,6 @@ PassOwnPtr<ArgumentDecoder> Connection::sendSyncMessage(MessageID messageID, uin
         m_pendingSyncReplies.append(PendingSyncReply(syncRequestID));
     }
 
-    // We have to begin waiting for the sync reply before sending the message, in case the other side
-    // would have sent a request before us, which would lead to a deadlock.
-    m_syncMessageState->beginWaitForSyncReply();
-
     // First send the message.
     messageID = messageID.messageIDWithAddedFlags(MessageID::DispatchMessageWhenWaitingForSyncReply | MessageID::SyncMessage);
     sendMessage(messageID, encoder);
@@ -377,8 +361,6 @@ PassOwnPtr<ArgumentDecoder> Connection::sendSyncMessage(MessageID messageID, uin
         m_pendingSyncReplies.removeLast();
     }
 
-    m_syncMessageState->endWaitForSyncReply();
-
     if (!reply && m_client)
         m_client->didFailToSendSyncMessage(this);