Bug 1240985 - Check for cancellation during ProcessPendingRequests (r=dvander)
☠☠ backed out by 10d4e8736dbb ☠ ☠
authorBill McCloskey <billm@mozilla.com>
Tue, 19 Jan 2016 17:34:31 -0800
changeset 280849 488690ba4c8fa9a09f55ca7a77614219f2a0c57a
parent 280848 a7eecc68ca20da93faf415819679236d3196491a
child 280850 3fcd50d8382165d89359b3a238b686a3624e35f4
push id29922
push usercbook@mozilla.com
push dateThu, 21 Jan 2016 10:51:00 +0000
treeherdermozilla-central@977d78a8dd78 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs1240985
milestone46.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1240985 - Check for cancellation during ProcessPendingRequests (r=dvander)
ipc/glue/MessageChannel.cpp
ipc/glue/MessageChannel.h
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -788,40 +788,60 @@ 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) {
                 if (!toProcess.append(Move(msg)))
                     MOZ_CRASH();
                 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;
@@ -934,42 +954,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);