Bug 1240985 - Check for cancellation during ProcessPendingRequests (r=dvander,a=sylvestre)
authorBill McCloskey <billm@mozilla.com>
Tue, 19 Jan 2016 17:34:31 -0800
changeset 310969 882c647d91038021697224227016af46be74e2b5
parent 310968 0e4c292ead72de790c7073eca00cc06de1fb142a
child 310970 20666a406bfad3b39ad70757bc2b1b2236a3ac5f
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander, sylvestre
bugs1240985
milestone45.0a2
Bug 1240985 - Check for cancellation during ProcessPendingRequests (r=dvander,a=sylvestre)
ipc/glue/MessageChannel.cpp
ipc/glue/MessageChannel.h
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -792,39 +792,59 @@ MessageChannel::OnMessageReceivedFromLin
             // If we compressed away the previous message, we'll re-use
             // its pending task.
             mWorkerLoop->PostTask(FROM_HERE, new DequeueTask(mDequeueOneTask));
         }
     }
 }
 
 void
-MessageChannel::ProcessPendingRequests()
+MessageChannel::ProcessPendingRequests(int transaction, int prio)
 {
+    IPC_LOG("ProcessPendingRequests");
+
     // Loop until there aren't any more priority messages to process.
     for (;;) {
         mozilla::Vector<Message> toProcess;
 
         for (MessageQueue::iterator it = mPending.begin(); it != mPending.end(); ) {
             Message &msg = *it;
-            if (!ShouldDeferMessage(msg)) {
+
+            bool defer = ShouldDeferMessage(msg);
+
+            // Only log the interesting messages.
+            if (msg.is_sync() || msg.priority() == IPC::Message::PRIORITY_URGENT) {
+                IPC_LOG("ShouldDeferMessage(seqno=%d) = %d", msg.seqno(), defer);
+            }
+
+            if (!defer) {
                 toProcess.append(Move(msg));
                 it = mPending.erase(it);
                 continue;
             }
             it++;
         }
 
         if (toProcess.empty())
             break;
 
         // Processing these messages could result in more messages, so we
         // loop around to check for more afterwards.
+
         for (auto it = toProcess.begin(); it != toProcess.end(); it++)
             ProcessPendingRequest(*it);
+
+        // If we canceled during ProcessPendingRequest, then we need to leave
+        // immediately because the results of ShouldDeferMessage will be
+        // operating with weird state (as if no Send is in progress). That could
+        // cause even normal priority sync messages to be processed (but not
+        // normal priority async messages), which would break message ordering.
+        if (WasTransactionCanceled(transaction, prio)) {
+            return;
+        }
     }
 }
 
 bool
 MessageChannel::WasTransactionCanceled(int transaction, int prio)
 {
     if (transaction == mCurrentTransaction) {
         return false;
@@ -937,42 +957,49 @@ MessageChannel::Send(Message* aMsg, Mess
 
     AutoSetValue<bool> replies(mAwaitingSyncReply, true);
     AutoSetValue<int> prioSet(mAwaitingSyncReplyPriority, prio);
     AutoEnterTransaction transact(this, seqno);
 
     int32_t transaction = mCurrentTransaction;
     msg->set_transaction_id(transaction);
 
-    ProcessPendingRequests();
+    IPC_LOG("Send seqno=%d, xid=%d", seqno, transaction);
+
+    ProcessPendingRequests(transaction, prio);
     if (WasTransactionCanceled(transaction, prio)) {
+        IPC_LOG("Other side canceled seqno=%d, xid=%d", seqno, transaction);
         return false;
     }
 
     bool handleWindowsMessages = mListener->HandleWindowsMessages(*aMsg);
     mLink->SendMessage(msg.forget());
 
     while (true) {
-        ProcessPendingRequests();
+        ProcessPendingRequests(transaction, prio);
         if (WasTransactionCanceled(transaction, prio)) {
+            IPC_LOG("Other side canceled seqno=%d, xid=%d", seqno, transaction);
             return false;
         }
 
         // See if we've received a reply.
         if (mRecvdErrors) {
+            IPC_LOG("Error: seqno=%d, xid=%d", seqno, transaction);
             mRecvdErrors--;
             return false;
         }
 
         if (mRecvd) {
+            IPC_LOG("Got reply: seqno=%d, xid=%d", seqno, transaction);
             break;
         }
 
         MOZ_ASSERT(!mTimedOutMessageSeqno);
 
+        MOZ_ASSERT(mCurrentTransaction == transaction);
         bool maybeTimedOut = !WaitForSyncNotify(handleWindowsMessages);
 
         if (!Connected()) {
             ReportConnectionError("MessageChannel::SendAndWait");
             return false;
         }
 
         if (WasTransactionCanceled(transaction, prio)) {
--- a/ipc/glue/MessageChannel.h
+++ b/ipc/glue/MessageChannel.h
@@ -245,17 +245,17 @@ class MessageChannel : HasResultCodes
     void Clear();
 
     // Send OnChannelConnected notification to listeners.
     void DispatchOnChannelConnected();
 
     bool InterruptEventOccurred();
     bool HasPendingEvents();
 
-    void ProcessPendingRequests();
+    void ProcessPendingRequests(int transaction, int prio);
     bool ProcessPendingRequest(const Message &aUrgent);
 
     void MaybeUndeferIncall();
     void EnqueuePendingMessages();
 
     // Executed on the worker thread. Dequeues one pending message.
     bool OnMaybeDequeueOne();
     bool DequeueOne(Message *recvd);