Title: [139477] branches/safari-536.28-branch/Source/WebKit2
Revision
139477
Author
ander...@apple.com
Date
2013-01-11 11:50:19 -0800 (Fri, 11 Jan 2013)

Log Message

Merge r139464.

    2013-01-11  Anders Carlsson  <ander...@apple.com>
    
    Incoming synchronous messages can sometimes arrive out of order
    https://bugs.webkit.org/show_bug.cgi?id=106677
    <rdar://problem/12889499>

    Reviewed by Andreas Kling.

    In cases where synchronous messages come in more than one connection at the same time, we can sometimes deliver
    the synchronous messages before any pending asynchronous messages on that connection. This breaks FIFO ordering.

    Fix this by separating the "dispatch all incoming synchronous messages" phase out into multiple phases, so we'll
    schedule one call per connection instead of one call for all connections.

    * Platform/CoreIPC/Connection.cpp:
    (Connection::SyncMessageState):
    (CoreIPC::Connection::SyncMessageState::SyncMessageState):
    (CoreIPC::Connection::SyncMessageState::processIncomingMessage):
    (CoreIPC::Connection::SyncMessageState::dispatchMessages):
    (CoreIPC::Connection::SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesForConnection):
    (CoreIPC::Connection::waitForSyncReply):

Modified Paths

Diff

Modified: branches/safari-536.28-branch/Source/WebKit2/ChangeLog (139476 => 139477)


--- branches/safari-536.28-branch/Source/WebKit2/ChangeLog	2013-01-11 19:49:31 UTC (rev 139476)
+++ branches/safari-536.28-branch/Source/WebKit2/ChangeLog	2013-01-11 19:50:19 UTC (rev 139477)
@@ -1,3 +1,29 @@
+2013-01-11  Anders Carlsson  <ander...@apple.com>
+
+        Merge r139464.
+
+    2013-01-11  Anders Carlsson  <ander...@apple.com>
+    
+            Incoming synchronous messages can sometimes arrive out of order
+            https://bugs.webkit.org/show_bug.cgi?id=106677
+            <rdar://problem/12889499>
+
+            Reviewed by Andreas Kling.
+
+            In cases where synchronous messages come in more than one connection at the same time, we can sometimes deliver
+            the synchronous messages before any pending asynchronous messages on that connection. This breaks FIFO ordering.
+
+            Fix this by separating the "dispatch all incoming synchronous messages" phase out into multiple phases, so we'll
+            schedule one call per connection instead of one call for all connections.
+
+            * Platform/CoreIPC/Connection.cpp:
+            (Connection::SyncMessageState):
+            (CoreIPC::Connection::SyncMessageState::SyncMessageState):
+            (CoreIPC::Connection::SyncMessageState::processIncomingMessage):
+            (CoreIPC::Connection::SyncMessageState::dispatchMessages):
+            (CoreIPC::Connection::SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesForConnection):
+            (CoreIPC::Connection::waitForSyncReply):
+
 2012-12-13  Lucas Forschler  <lforsch...@apple.com>
 
     Rollout r133937

Modified: branches/safari-536.28-branch/Source/WebKit2/Platform/CoreIPC/Connection.cpp (139476 => 139477)


--- branches/safari-536.28-branch/Source/WebKit2/Platform/CoreIPC/Connection.cpp	2013-01-11 19:49:31 UTC (rev 139476)
+++ branches/safari-536.28-branch/Source/WebKit2/Platform/CoreIPC/Connection.cpp	2013-01-11 19:50:19 UTC (rev 139477)
@@ -30,6 +30,7 @@
 #include "CoreIPCMessageKinds.h"
 #include <WebCore/RunLoop.h>
 #include <wtf/CurrentTime.h>
+#include <wtf/HashSet.h>
 
 using namespace std;
 using namespace WebCore;
@@ -62,7 +63,9 @@
     // waiting for a reply to a synchronous message.
     bool processIncomingMessage(Connection*, IncomingMessage&);
 
-    void dispatchMessages();
+    // Dispatch pending sync messages. if allowedConnection is not null, will only dispatch messages
+    // from that connection and put the other messages back in the queue.
+    void dispatchMessages(Connection* allowedConnection);
 
 private:
     explicit SyncMessageState(RunLoop*);
@@ -80,15 +83,16 @@
         return syncMessageStateMapMutex;
     }
 
-    void dispatchMessageAndResetDidScheduleDispatchMessagesWork();
+    void dispatchMessageAndResetDidScheduleDispatchMessagesForConnection(Connection*);
 
     RunLoop* m_runLoop;
     BinarySemaphore m_waitForSyncReplySemaphore;
 
-    // Protects m_didScheduleDispatchMessagesWork and m_messagesToDispatchWhileWaitingForSyncReply.
+    // Protects m_didScheduleDispatchMessagesWorkSet and m_messagesToDispatchWhileWaitingForSyncReply.
     Mutex m_mutex;
 
-    bool m_didScheduleDispatchMessagesWork;
+    // The set of connections for which we've scheduled a call to dispatchMessageAndResetDidScheduleDispatchMessagesForConnection.
+    HashSet<RefPtr<Connection> > m_didScheduleDispatchMessagesWorkSet;
 
     struct ConnectionAndIncomingMessage {
         RefPtr<Connection> connection;
@@ -115,7 +119,6 @@
 
 Connection::SyncMessageState::SyncMessageState(RunLoop* runLoop)
     : m_runLoop(runLoop)
-    , m_didScheduleDispatchMessagesWork(false)
 {
 }
 
@@ -139,12 +142,10 @@
 
     {
         MutexLocker locker(m_mutex);
-        
-        if (!m_didScheduleDispatchMessagesWork) {
-            m_runLoop->dispatch(bind(&SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesWork, this));
-            m_didScheduleDispatchMessagesWork = true;
-        }
 
+        if (m_didScheduleDispatchMessagesWorkSet.add(connection).isNewEntry)
+            m_runLoop->dispatch(bind(&SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesForConnection, this, RefPtr<Connection>(connection)));
+
         m_messagesToDispatchWhileWaitingForSyncReply.append(connectionAndIncomingMessage);
     }
 
@@ -153,7 +154,7 @@
     return true;
 }
 
-void Connection::SyncMessageState::dispatchMessages()
+void Connection::SyncMessageState::dispatchMessages(Connection* allowedConnection)
 {
     ASSERT(m_runLoop == RunLoop::current());
 
@@ -164,21 +165,36 @@
         m_messagesToDispatchWhileWaitingForSyncReply.swap(messagesToDispatchWhileWaitingForSyncReply);
     }
 
+    Vector<ConnectionAndIncomingMessage> messagesToPutBack;
+
     for (size_t i = 0; i < messagesToDispatchWhileWaitingForSyncReply.size(); ++i) {
         ConnectionAndIncomingMessage& connectionAndIncomingMessage = messagesToDispatchWhileWaitingForSyncReply[i];
+
+        if (allowedConnection && allowedConnection != connectionAndIncomingMessage.connection) {
+            // This incoming message belongs to another connection and we don't want to dispatch it now
+            // so mark it to be put back in the message queue.
+            messagesToPutBack.append(connectionAndIncomingMessage);
+            continue;
+        }
+
         connectionAndIncomingMessage.connection->dispatchMessage(connectionAndIncomingMessage.incomingMessage);
     }
+
+    if (!messagesToPutBack.isEmpty()) {
+        MutexLocker locker(m_mutex);
+        m_messagesToDispatchWhileWaitingForSyncReply.append(messagesToPutBack);
+    }
 }
 
-void Connection::SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesWork()
+void Connection::SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesForConnection(Connection* connection)
 {
     {
         MutexLocker locker(m_mutex);
-        ASSERT(m_didScheduleDispatchMessagesWork);
-        m_didScheduleDispatchMessagesWork = false;
+        ASSERT(m_didScheduleDispatchMessagesWorkSet.contains(connection));
+        m_didScheduleDispatchMessagesWorkSet.remove(connection);
     }
 
-    dispatchMessages();
+    dispatchMessages(connection);
 }
 
 PassRefPtr<Connection> Connection::createServerConnection(Identifier identifier, Client* client, RunLoop* clientRunLoop)
@@ -432,7 +448,7 @@
     bool timedOut = false;
     while (!timedOut) {
         // First, check if we have any messages that we need to process.
-        m_syncMessageState->dispatchMessages();
+        m_syncMessageState->dispatchMessages(0);
         
         {
             MutexLocker locker(m_syncReplyStateMutex);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to