Bug 1177013 - CancelCurrentTransaction IPC support. r=dvander, a=ritu
authorBill McCloskey <billm@mozilla.com>
Fri, 19 Jun 2015 11:32:35 -0700
changeset 281680 e4e856f1d56f09746c49f5176055e17ddcd0c7e4
parent 281679 bdc01d55b9748abd69c45e11115f3799740f35bd
child 281681 a237339bf4c8dd1f3eac8a6451160742449e5ffb
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander, ritu
bugs1177013
milestone41.0a2
Bug 1177013 - CancelCurrentTransaction IPC support. r=dvander, a=ritu
ipc/glue/MessageChannel.cpp
ipc/glue/MessageChannel.h
ipc/glue/ProtocolUtils.h
ipc/ipdl/test/cxx/PTestCancel.ipdl
ipc/ipdl/test/cxx/TestCancel.cpp
ipc/ipdl/test/cxx/TestCancel.h
ipc/ipdl/test/cxx/moz.build
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -553,27 +553,31 @@ MessageChannel::Send(Message* aMsg)
 }
 
 bool
 MessageChannel::MaybeInterceptSpecialIOMessage(const Message& aMsg)
 {
     AssertLinkThread();
     mMonitor->AssertCurrentThreadOwns();
 
-    if (MSG_ROUTING_NONE == aMsg.routing_id() &&
-        GOODBYE_MESSAGE_TYPE == aMsg.type())
-    {
-        // :TODO: Sort out Close() on this side racing with Close() on the
-        // other side
-        mChannelState = ChannelClosing;
-        if (LoggingEnabled()) {
-            printf("NOTE: %s process received `Goodbye', closing down\n",
-                   (mSide == ChildSide) ? "child" : "parent");
+    if (MSG_ROUTING_NONE == aMsg.routing_id()) {
+        if (GOODBYE_MESSAGE_TYPE == aMsg.type()) {
+            // :TODO: Sort out Close() on this side racing with Close() on the
+            // other side
+            mChannelState = ChannelClosing;
+            if (LoggingEnabled()) {
+                printf("NOTE: %s process received `Goodbye', closing down\n",
+                       (mSide == ChildSide) ? "child" : "parent");
+            }
+            return true;
+        } else if (CANCEL_MESSAGE_TYPE == aMsg.type()) {
+            CancelCurrentTransactionInternal();
+            NotifyWorkerThread();
+            return true;
         }
-        return true;
     }
     return false;
 }
 
 bool
 MessageChannel::ShouldDeferMessage(const Message& aMsg)
 {
     // Never defer messages that have the highest priority, even async
@@ -638,16 +642,17 @@ MessageChannel::OnMessageReceivedFromLin
     // we know that it needs to be immediately handled to unblock us.
     if (aMsg.is_sync() && aMsg.is_reply()) {
         if (aMsg.seqno() == mTimedOutMessageSeqno) {
             // Drop the message, but allow future sync messages to be sent.
             mTimedOutMessageSeqno = 0;
             return;
         }
 
+        MOZ_ASSERT(aMsg.transaction_id() == mCurrentTransaction);
         MOZ_ASSERT(AwaitingSyncReply());
         MOZ_ASSERT(!mRecvd);
 
         // Rather than storing errors in mRecvd, we mark them in
         // mRecvdErrors. We need a counter because multiple replies can arrive
         // when a timeout happens, as in the following example. Imagine the
         // child is running slowly. The parent sends a sync message P1. It times
         // out. The child eventually sends a sync message C1. While waiting for
@@ -801,17 +806,17 @@ MessageChannel::Send(Message* aMsg, Mess
         // Don't allow sending CPOWs while we're dispatching a sync message.
         // If you want to do that, use sendRpcMessage instead.
         return false;
     }
 
     IPC_ASSERT(aMsg->is_sync(), "can only Send() sync messages here");
     IPC_ASSERT(aMsg->priority() >= DispatchingSyncMessagePriority(),
                "can't send sync message of a lesser priority than what's being dispatched");
-    IPC_ASSERT(mAwaitingSyncReplyPriority <= aMsg->priority(),
+    IPC_ASSERT(AwaitingSyncReplyPriority() <= aMsg->priority(),
                "nested sync message sends must be of increasing priority");
 
     IPC_ASSERT(DispatchingSyncMessagePriority() != IPC::Message::PRIORITY_URGENT,
                "not allowed to send messages while dispatching urgent messages");
     IPC_ASSERT(DispatchingAsyncMessagePriority() != IPC::Message::PRIORITY_URGENT,
                "not allowed to send messages while dispatching urgent messages");
 
     nsAutoPtr<Message> msg(aMsg);
@@ -830,21 +835,29 @@ 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();
+    if (mCurrentTransaction != transaction) {
+        // Transaction was canceled when dispatching.
+        return false;
+    }
 
     mLink->SendMessage(msg.forget());
 
     while (true) {
         ProcessPendingRequests();
+        if (mCurrentTransaction != transaction) {
+            // Transaction was canceled when dispatching.
+            return false;
+        }
 
         // See if we've received a reply.
         if (mRecvdErrors) {
             mRecvdErrors--;
             return false;
         }
 
         if (mRecvd) {
@@ -855,16 +868,21 @@ MessageChannel::Send(Message* aMsg, Mess
 
         bool maybeTimedOut = !WaitForSyncNotify();
 
         if (!Connected()) {
             ReportConnectionError("MessageChannel::SendAndWait");
             return false;
         }
 
+        if (mCurrentTransaction != transaction) {
+            // Transaction was canceled by other side.
+            return false;
+        }
+
         // We only time out a message if it initiated a new transaction (i.e.,
         // if neither side has any other message Sends on the stack).
         bool canTimeOut = transaction == seqno;
         if (maybeTimedOut && canTimeOut && !ShouldContinueFromTimeout()) {
             // We might have received a reply during WaitForSyncNotify or inside
             // ShouldContinueFromTimeout (which drops the lock). We need to make
             // sure not to set mTimedOutMessageSeqno if that happens, since then
             // there would be no way to unset it.
@@ -1180,25 +1198,36 @@ MessageChannel::DispatchMessage(const Me
     Maybe<AutoNoJSAPI> nojsapi;
     if (ScriptSettingsInitialized() && NS_IsMainThread())
         nojsapi.emplace();
 
     nsAutoPtr<Message> reply;
 
     {
         AutoEnterTransaction transaction(this, aMsg);
-        MonitorAutoUnlock unlock(*mMonitor);
-        CxxStackFrame frame(*this, IN_MESSAGE, &aMsg);
+
+        int id = aMsg.transaction_id();
+        MOZ_ASSERT_IF(aMsg.is_sync(), id == mCurrentTransaction);
+
+        {
+            MonitorAutoUnlock unlock(*mMonitor);
+            CxxStackFrame frame(*this, IN_MESSAGE, &aMsg);
 
-        if (aMsg.is_sync())
-            DispatchSyncMessage(aMsg, *getter_Transfers(reply));
-        else if (aMsg.is_interrupt())
-            DispatchInterruptMessage(aMsg, 0);
-        else
-            DispatchAsyncMessage(aMsg);
+            if (aMsg.is_sync())
+                DispatchSyncMessage(aMsg, *getter_Transfers(reply));
+            else if (aMsg.is_interrupt())
+                DispatchInterruptMessage(aMsg, 0);
+            else
+                DispatchAsyncMessage(aMsg);
+        }
+
+        if (mCurrentTransaction != id) {
+            // The transaction has been canceled. Don't send a reply.
+            reply = nullptr;
+        }
     }
 
     if (reply && ChannelConnected == mChannelState) {
         mLink->SendMessage(reply.forget());
     }
 }
 
 void
@@ -1210,17 +1239,17 @@ MessageChannel::DispatchSyncMessage(cons
 
     // We don't want to run any code that might run a nested event loop here, so
     // we avoid running event handlers. Once we've sent the response to the
     // urgent message, it's okay to run event handlers again since the parent is
     // no longer blocked.
     MOZ_ASSERT_IF(prio > IPC::Message::PRIORITY_NORMAL, NS_IsMainThread());
     MaybeScriptBlocker scriptBlocker(this, prio > IPC::Message::PRIORITY_NORMAL);
 
-    IPC_ASSERT(prio >= mDispatchingSyncMessagePriority,
+    IPC_ASSERT(prio >= DispatchingSyncMessagePriority(),
                "priority inversion while dispatching sync message");
     IPC_ASSERT(prio >= mAwaitingSyncReplyPriority,
                "dispatching a message of lower priority while waiting for a response");
 
     bool dummy;
     bool& blockingVar = ShouldBlockScripts() ? gParentIsBlocked : dummy;
 
     Result rv;
@@ -1903,16 +1932,56 @@ MessageChannel::GetTopmostMessageRouting
     MOZ_ASSERT(MessageLoop::current() == mWorkerLoop);
     if (mCxxStackFrames.empty()) {
         return MSG_ROUTING_NONE;
     }
     const InterruptFrame& frame = mCxxStackFrames.back();
     return frame.GetRoutingId();
 }
 
+class CancelMessage : public IPC::Message
+{
+public:
+    CancelMessage() :
+        IPC::Message(MSG_ROUTING_NONE, CANCEL_MESSAGE_TYPE, PRIORITY_NORMAL)
+    {
+    }
+    static bool Read(const Message* msg) {
+        return true;
+    }
+    void Log(const std::string& aPrefix, FILE* aOutf) const {
+        fputs("(special `Cancel' message)", aOutf);
+    }
+};
+
+void
+MessageChannel::CancelCurrentTransactionInternal()
+{
+    // When we cancel a transaction, we need to behave as if there's no longer
+    // any IPC on the stack. Anything we were dispatching or sending will get
+    // canceled. Consequently, we have to update the state variables below.
+    //
+    // We also need to ensure that when any IPC functions on the stack return,
+    // they don't reset these values using an RAII class like AutoSetValue. To
+    // avoid that, these RAII classes check if the variable they set has been
+    // tampered with (by us). If so, they don't reset the variable to the old
+    // value.
+
+    MOZ_ASSERT(!mCurrentTransaction);
+    mCurrentTransaction = 0;
+}
+
+void
+MessageChannel::CancelCurrentTransaction()
+{
+    MonitorAutoLock lock(*mMonitor);
+    CancelCurrentTransactionInternal();
+    mLink->SendMessage(new CancelMessage());
+}
+
 bool
 ParentProcessIsBlocked()
 {
     return gParentIsBlocked;
 }
 
 } // ipc
 } // mozilla
--- a/ipc/glue/MessageChannel.h
+++ b/ipc/glue/MessageChannel.h
@@ -130,16 +130,18 @@ class MessageChannel : HasResultCodes
     bool CanSend() const;
 
     void SetReplyTimeoutMs(int32_t aTimeoutMs);
 
     bool IsOnCxxStack() const {
         return !mCxxStackFrames.empty();
     }
 
+    void CancelCurrentTransaction();
+
     /**
      * This function is used by hang annotation code to determine which IPDL
      * actor is highest in the call stack at the time of the hang. It should
      * be called from the main thread when a sync or intr message is about to
      * be sent.
      */
     int32_t GetTopmostMessageRoutingId() const;
 
@@ -260,16 +262,18 @@ class MessageChannel : HasResultCodes
     // necessarily.
     bool WaitForSyncNotify();
     bool WaitForInterruptNotify();
 
     bool WaitResponse(bool aWaitTimedOut);
 
     bool ShouldContinueFromTimeout();
 
+    void CancelCurrentTransactionInternal();
+
     // The "remote view of stack depth" can be different than the
     // actual stack depth when there are out-of-turn replies.  When we
     // receive one, our actual Interrupt stack depth doesn't decrease, but
     // the other side (that sent the reply) thinks it has.  So, the
     // "view" returned here is |stackDepth| minus the number of
     // out-of-turn replies.
     //
     // Only called from the worker thread.
@@ -541,46 +545,52 @@ class MessageChannel : HasResultCodes
     // To ensure IDs are unique, we use sequence numbers for transaction IDs,
     // which grow in opposite directions from child to parent.
 
     // The current transaction ID.
     int32_t mCurrentTransaction;
 
     class AutoEnterTransaction
     {
-      public:
+     public:
        explicit AutoEnterTransaction(MessageChannel *aChan, int32_t aMsgSeqno)
         : mChan(aChan),
+          mNewTransaction(0),
           mOldTransaction(mChan->mCurrentTransaction)
        {
            mChan->mMonitor->AssertCurrentThreadOwns();
-           if (mChan->mCurrentTransaction == 0)
+           if (mChan->mCurrentTransaction == 0) {
+               mNewTransaction = aMsgSeqno;
                mChan->mCurrentTransaction = aMsgSeqno;
+           }
        }
        explicit AutoEnterTransaction(MessageChannel *aChan, const Message &aMessage)
         : mChan(aChan),
+          mNewTransaction(aMessage.transaction_id()),
           mOldTransaction(mChan->mCurrentTransaction)
        {
            mChan->mMonitor->AssertCurrentThreadOwns();
 
            if (!aMessage.is_sync())
                return;
 
            MOZ_ASSERT_IF(mChan->mSide == ParentSide && mOldTransaction != aMessage.transaction_id(),
                          !mOldTransaction || aMessage.priority() > mChan->AwaitingSyncReplyPriority());
            mChan->mCurrentTransaction = aMessage.transaction_id();
        }
        ~AutoEnterTransaction() {
            mChan->mMonitor->AssertCurrentThreadOwns();
-           mChan->mCurrentTransaction = mOldTransaction;
+           if (mChan->mCurrentTransaction == mNewTransaction) {
+               mChan->mCurrentTransaction = mOldTransaction;
+           }
        }
 
       private:
        MessageChannel *mChan;
-       int32_t mOldTransaction;
+       int32_t mNewTransaction, mOldTransaction;
     };
 
     // If a sync message times out, we store its sequence number here. Any
     // future sync messages will fail immediately. Once the reply for original
     // sync message is received, we allow sync messages again.
     //
     // When a message times out, nothing is done to inform the other side. The
     // other side will eventually dispatch the message and send a reply. Our
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -33,20 +33,21 @@
 namespace {
 // XXX the max message ID is actually kuint32max now ... when this
 // changed, the assumptions of the special message IDs changed in that
 // they're not carving out messages from likely-unallocated space, but
 // rather carving out messages from the end of space allocated to
 // protocol 0.  Oops!  We can get away with this until protocol 0
 // starts approaching its 65,536th message.
 enum {
-    CHANNEL_OPENED_MESSAGE_TYPE = kuint16max - 5,
-    SHMEM_DESTROYED_MESSAGE_TYPE = kuint16max - 4,
-    SHMEM_CREATED_MESSAGE_TYPE = kuint16max - 3,
-    GOODBYE_MESSAGE_TYPE       = kuint16max - 2
+    CHANNEL_OPENED_MESSAGE_TYPE = kuint16max - 6,
+    SHMEM_DESTROYED_MESSAGE_TYPE = kuint16max - 5,
+    SHMEM_CREATED_MESSAGE_TYPE = kuint16max - 4,
+    GOODBYE_MESSAGE_TYPE       = kuint16max - 3,
+    CANCEL_MESSAGE_TYPE        = kuint16max - 2,
 
     // kuint16max - 1 is used by ipc_channel.h.
 };
 }
 
 namespace mozilla {
 namespace dom {
 class ContentParent;
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/cxx/PTestCancel.ipdl
@@ -0,0 +1,36 @@
+namespace mozilla {
+namespace _ipdltest {
+
+prio(normal upto high) sync protocol PTestCancel
+{
+// Test1
+child:
+    prio(high) sync Test1_1();
+parent:
+    async Done1();
+
+// Test2
+child:
+    async Start2();
+    prio(high) sync Test2_2();
+parent:
+    sync Test2_1();
+
+// Test3
+child:
+    prio(high) sync Test3_1();
+parent:
+    async Start3();
+    prio(high) sync Test3_2();
+
+parent:
+    async Done();
+
+child:
+    prio(high) sync CheckChild() returns (uint32_t reply);
+parent:
+    prio(high) sync CheckParent() returns (uint32_t reply);
+};
+
+} // namespace _ipdltest
+} // namespace mozilla
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/cxx/TestCancel.cpp
@@ -0,0 +1,175 @@
+#include "TestCancel.h"
+
+#include "IPDLUnitTests.h"      // fail etc.
+
+template<>
+struct RunnableMethodTraits<mozilla::_ipdltest::TestCancelParent>
+{
+    static void RetainCallee(mozilla::_ipdltest::TestCancelParent* obj) { }
+    static void ReleaseCallee(mozilla::_ipdltest::TestCancelParent* obj) { }
+};
+
+namespace mozilla {
+namespace _ipdltest {
+
+//-----------------------------------------------------------------------------
+// parent
+
+TestCancelParent::TestCancelParent()
+{
+    MOZ_COUNT_CTOR(TestCancelParent);
+}
+
+TestCancelParent::~TestCancelParent()
+{
+    MOZ_COUNT_DTOR(TestCancelParent);
+}
+
+void
+TestCancelParent::Main()
+{
+    if (SendTest1_1())
+	fail("sending Test1_1");
+
+    uint32_t value = 0;
+    if (!SendCheckChild(&value))
+	fail("Test1 CheckChild");
+
+    if (value != 12)
+	fail("Test1 CheckChild reply");
+}
+
+bool
+TestCancelParent::RecvDone1()
+{
+    if (!SendStart2())
+	fail("sending Start2");
+
+    return true;
+}
+
+bool
+TestCancelParent::RecvTest2_1()
+{
+    if (SendTest2_2())
+	fail("sending Test2_2");
+
+    return true;
+}
+
+bool
+TestCancelParent::RecvStart3()
+{
+    if (SendTest3_1())
+	fail("sending Test3_1");
+
+    uint32_t value = 0;
+    if (!SendCheckChild(&value))
+	fail("Test1 CheckChild");
+
+    if (value != 12)
+	fail("Test1 CheckChild reply");
+
+    return true;
+}
+
+bool
+TestCancelParent::RecvTest3_2()
+{
+    GetIPCChannel()->CancelCurrentTransaction();
+    return true;
+}
+
+bool
+TestCancelParent::RecvDone()
+{
+    MessageLoop::current()->PostTask(
+	FROM_HERE, NewRunnableMethod(this, &TestCancelParent::Close));
+    return true;
+}
+
+bool
+TestCancelParent::RecvCheckParent(uint32_t *reply)
+{
+    *reply = 12;
+    return true;
+}
+
+//-----------------------------------------------------------------------------
+// child
+
+bool
+TestCancelChild::RecvTest1_1()
+{
+    GetIPCChannel()->CancelCurrentTransaction();
+
+    uint32_t value = 0;
+    if (!SendCheckParent(&value))
+	fail("Test1 CheckParent");
+
+    if (value != 12)
+	fail("Test1 CheckParent reply");
+
+    if (!SendDone1())
+	fail("Test1 CheckParent");
+
+    return true;
+}
+
+bool
+TestCancelChild::RecvStart2()
+{
+    if (!SendTest2_1())
+	fail("sending Test2_1");
+
+    if (!SendStart3())
+	fail("sending Start3");
+
+    return true;
+}
+
+bool
+TestCancelChild::RecvTest2_2()
+{
+    GetIPCChannel()->CancelCurrentTransaction();
+    return true;
+}
+
+bool
+TestCancelChild::RecvTest3_1()
+{
+    if (SendTest3_2())
+	fail("sending Test3_2");
+
+    uint32_t value = 0;
+    if (!SendCheckParent(&value))
+	fail("Test1 CheckParent");
+
+    if (value != 12)
+	fail("Test1 CheckParent reply");
+
+    if (!SendDone())
+	fail("sending Done");
+
+    return true;
+}
+
+bool
+TestCancelChild::RecvCheckChild(uint32_t *reply)
+{
+    *reply = 12;
+    return true;
+}
+
+TestCancelChild::TestCancelChild()
+{
+    MOZ_COUNT_CTOR(TestCancelChild);
+}
+
+TestCancelChild::~TestCancelChild()
+{
+    MOZ_COUNT_DTOR(TestCancelChild);
+}
+
+} // namespace _ipdltest
+} // namespace mozilla
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/cxx/TestCancel.h
@@ -0,0 +1,66 @@
+#ifndef mozilla__ipdltest_TestCancel_h
+#define mozilla__ipdltest_TestCancel_h 1
+
+#include "mozilla/_ipdltest/IPDLUnitTests.h"
+
+#include "mozilla/_ipdltest/PTestCancelParent.h"
+#include "mozilla/_ipdltest/PTestCancelChild.h"
+
+namespace mozilla {
+namespace _ipdltest {
+
+
+class TestCancelParent :
+    public PTestCancelParent
+{
+public:
+    TestCancelParent();
+    virtual ~TestCancelParent();
+
+    static bool RunTestInProcesses() { return true; }
+    static bool RunTestInThreads() { return false; }
+
+    void Main();
+
+    virtual bool RecvDone1() override;
+    virtual bool RecvTest2_1() override;
+    virtual bool RecvStart3() override;
+    virtual bool RecvTest3_2() override;
+    virtual bool RecvDone() override;
+
+    virtual bool RecvCheckParent(uint32_t *reply) override;
+
+    virtual void ActorDestroy(ActorDestroyReason why) override
+    {
+        passed("ok");
+        QuitParent();
+    }
+};
+
+
+class TestCancelChild :
+    public PTestCancelChild
+{
+public:
+    TestCancelChild();
+    virtual ~TestCancelChild();
+
+    virtual bool RecvTest1_1() override;
+    virtual bool RecvStart2() override;
+    virtual bool RecvTest2_2() override;
+    virtual bool RecvTest3_1() override;
+
+    virtual bool RecvCheckChild(uint32_t *reply) override;
+
+    virtual void ActorDestroy(ActorDestroyReason why) override
+    {
+        QuitChild();
+    }
+};
+
+
+} // namespace _ipdltest
+} // namespace mozilla
+
+
+#endif // ifndef mozilla__ipdltest_TestCancel_h
--- a/ipc/ipdl/test/cxx/moz.build
+++ b/ipc/ipdl/test/cxx/moz.build
@@ -12,16 +12,17 @@ EXPORTS.mozilla._ipdltest += [
     'IPDLUnitTestTypes.h',
     'IPDLUnitTestUtils.h',
 ]
 
 SOURCES += [
     'TestActorPunning.cpp',
     'TestBadActor.cpp',
     'TestBridgeMain.cpp',
+    'TestCancel.cpp',
     'TestCrashCleanup.cpp',
     'TestDataStructures.cpp',
     'TestDesc.cpp',
     'TestFailedCtor.cpp',
     'TestHangs.cpp',
     'TestHighestPrio.cpp',
     'TestInterruptErrorCleanup.cpp',
     'TestInterruptRaces.cpp',
@@ -65,16 +66,17 @@ IPDL_SOURCES += [
     'PTestActorPunning.ipdl',
     'PTestActorPunningPunned.ipdl',
     'PTestActorPunningSub.ipdl',
     'PTestBadActor.ipdl',
     'PTestBadActorSub.ipdl',
     'PTestBridgeMain.ipdl',
     'PTestBridgeMainSub.ipdl',
     'PTestBridgeSub.ipdl',
+    'PTestCancel.ipdl',
     'PTestCrashCleanup.ipdl',
     'PTestDataStructures.ipdl',
     'PTestDataStructuresCommon.ipdlh',
     'PTestDataStructuresSub.ipdl',
     'PTestDesc.ipdl',
     'PTestDescSub.ipdl',
     'PTestDescSubsub.ipdl',
     'PTestFailedCtor.ipdl',