Bug 1177013 - CancelCurrentTransaction IPC support (r=dvander)

ipc/glue/MessageChannel.cpp

@@ -558,9 +558,8 @@ MessageChannel::MaybeInterceptSpecialIOMessage(const Message& aMsg)
     AssertLinkThread();
     mMonitor->AssertCurrentThreadOwns();
 
-    if (MSG_ROUTING_NONE == aMsg.routing_id() &&
-        GOODBYE_MESSAGE_TYPE == aMsg.type())
-    {
+    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;
@@ -569,6 +568,11 @@ MessageChannel::MaybeInterceptSpecialIOMessage(const Message& aMsg)
                        (mSide == ChildSide) ? "child" : "parent");
             }
             return true;
+        } else if (CANCEL_MESSAGE_TYPE == aMsg.type()) {
+            CancelCurrentTransactionInternal();
+            NotifyWorkerThread();
+            return true;
+        }
     }
     return false;
 }
@@ -643,6 +647,7 @@ MessageChannel::OnMessageReceivedFromLink(const Message& aMsg)
             return;
         }
 
+        MOZ_ASSERT(aMsg.transaction_id() == mCurrentTransaction);
         MOZ_ASSERT(AwaitingSyncReply());
         MOZ_ASSERT(!mRecvd);
 
@@ -806,7 +811,7 @@ MessageChannel::Send(Message* aMsg, Message* aReply)
     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,
@@ -835,11 +840,19 @@ MessageChannel::Send(Message* aMsg, Message* aReply)
     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) {
@@ -860,6 +873,11 @@ MessageChannel::Send(Message* aMsg, Message* aReply)
             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;
@@ -1185,6 +1203,11 @@ MessageChannel::DispatchMessage(const Message &aMsg)
 
     {
         AutoEnterTransaction transaction(this, aMsg);
+
+        int id = aMsg.transaction_id();
+        MOZ_ASSERT_IF(aMsg.is_sync(), id == mCurrentTransaction);
+
+        {
             MonitorAutoUnlock unlock(*mMonitor);
             CxxStackFrame frame(*this, IN_MESSAGE, &aMsg);
 
@@ -1196,6 +1219,12 @@ MessageChannel::DispatchMessage(const Message &aMsg)
                 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());
     }
@@ -1215,7 +1244,7 @@ MessageChannel::DispatchSyncMessage(const Message& aMsg, Message*& aReply)
     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");
@@ -1911,6 +1940,46 @@ MessageChannel::GetTopmostMessageRoutingId() const
     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()
 {

ipc/glue/MessageChannel.h

@@ -135,6 +135,8 @@ class MessageChannel : HasResultCodes
         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
@@ -265,6 +267,8 @@ class MessageChannel : HasResultCodes
 
     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
@@ -549,14 +553,18 @@ class MessageChannel : HasResultCodes
      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();
@@ -570,12 +578,14 @@ class MessageChannel : HasResultCodes
        }
        ~AutoEnterTransaction() {
            mChan->mMonitor->AssertCurrentThreadOwns();
+           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

ipc/glue/ProtocolUtils.h

@@ -38,10 +38,11 @@ namespace {
 // 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.
 };

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

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

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

ipc/ipdl/test/cxx/moz.build

@@ -17,6 +17,7 @@ SOURCES += [
     'TestActorPunning.cpp',
     'TestBadActor.cpp',
     'TestBridgeMain.cpp',
+    'TestCancel.cpp',
     'TestCrashCleanup.cpp',
     'TestDataStructures.cpp',
     'TestDesc.cpp',
@@ -69,6 +70,7 @@ IPDL_SOURCES += [
     'PTestBridgeMain.ipdl',
     'PTestBridgeMainSub.ipdl',
     'PTestBridgeSub.ipdl',
+    'PTestCancel.ipdl',
     'PTestCrashCleanup.ipdl',
     'PTestDataStructures.ipdl',
     'PTestDataStructuresCommon.ipdlh',