Bug 1364857 - Reject pending promises for actor when it's going to be destroyed. r=kanru

The lifetime of async IPDL returned promise may be longer than its actor.
That is, the handler (receiver) may have not resolve/reject the promise when the actor
is destroyed. In this case, we have to reject all the pending promises before
ActorDestroy() is called on the "sender" side.

Besides, the handler (receiver) can reject with reason "ActorDestroyed" to silently
cancel the promise without trying to reply to the remote actor which may
have died. The sender-side promise is responsible for rejecting the pending promises,
which will be done in MessageChannel::RejectPendingPromisesForActor().

MozReview-Commit-ID: 4XjmquZzDBO

--HG--
extra : rebase_source : 48539e35e4587e09be1d66497b1ea32d1a95ee9a

ipc/glue/MessageChannel.cpp

@@ -717,7 +717,9 @@ MessageChannel::Clear()
 
     gUnresolvedPromises -= mPendingPromises.size();
     for (auto& pair : mPendingPromises) {
-        pair.second.mRejectFunction(pair.second.mPromise, __func__);
+        pair.second.mRejectFunction(pair.second.mPromise,
+                                    PromiseRejectReason::ChannelClosed,
+                                    __func__);
     }
     mPendingPromises.clear();
 
@@ -944,6 +946,26 @@ MessageChannel::PopPromise(const Message& aMsg)
     return nullptr;
 }
 
+void
+MessageChannel::RejectPendingPromisesForActor(ActorIdType aActorId)
+{
+  auto itr = mPendingPromises.begin();
+  while (itr != mPendingPromises.end()) {
+    if (itr->second.mActorId != aActorId) {
+      ++itr;
+      continue;
+    }
+    auto& promise = itr->second.mPromise;
+    itr->second.mRejectFunction(promise,
+                                PromiseRejectReason::ActorDestroyed,
+                                __func__);
+    // Take special care of advancing the iterator since we are
+    // removing it while iterating.
+    itr = mPendingPromises.erase(itr);
+    gUnresolvedPromises--;
+  }
+}
+
 class BuildIDMessage : public IPC::Message
 {
 public:

ipc/glue/MessageChannel.h

@@ -69,6 +69,7 @@ enum class PromiseRejectReason {
     SendError,
     ChannelClosed,
     HandlerRejected,
+    ActorDestroyed,
     EndGuard_,
 };
 
@@ -93,10 +94,22 @@ class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver
 
     typedef mozilla::Monitor Monitor;
 
+    // We could templatize the actor type but it would unnecessarily
+    // expand the code size. Using the actor address as the
+    // identifier is already good enough.
+    typedef void* ActorIdType;
+
     struct PromiseHolder
     {
         RefPtr<MozPromiseRefcountable> mPromise;
-        std::function<void(MozPromiseRefcountable*, const char*)> mRejectFunction;
+
+        // For rejecting and removing the pending promises when a
+        // subprotocol is destoryed.
+        ActorIdType mActorId;
+
+        std::function<void(MozPromiseRefcountable*,
+                           PromiseRejectReason,
+                           const char*)> mRejectFunction;
     };
     static Atomic<size_t> gUnresolvedPromises;
     friend class PromiseReporter;
@@ -176,7 +189,7 @@ class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver
     // Asynchronously send a message to the other side of the channel
     // and wait for asynchronous reply
     template<typename Promise>
-    bool Send(Message* aMsg, Promise* aPromise) {
+    bool Send(Message* aMsg, Promise* aPromise, ActorIdType aActorId) {
         int32_t seqno = NextSeqno();
         aMsg->set_seqno(seqno);
         if (!Send(aMsg)) {
@@ -184,10 +197,11 @@ class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver
         }
         PromiseHolder holder;
         holder.mPromise = aPromise;
+        holder.mActorId = aActorId;
         holder.mRejectFunction = [](MozPromiseRefcountable* aRejectPromise,
+                                    PromiseRejectReason aReason,
                                     const char* aRejectSite) {
-            static_cast<Promise*>(aRejectPromise)->Reject(
-                PromiseRejectReason::ChannelClosed, aRejectSite);
+            static_cast<Promise*>(aRejectPromise)->Reject(aReason, aRejectSite);
         };
         mPendingPromises.insert(std::make_pair(seqno, Move(holder)));
         gUnresolvedPromises++;
@@ -214,6 +228,10 @@ class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver
     // Remove and return a promise that needs reply
     already_AddRefed<MozPromiseRefcountable> PopPromise(const Message& aMsg);
 
+    // Used to reject and remove pending promises owned by the given
+    // actor when it's about to be destroyed.
+    void RejectPendingPromisesForActor(ActorIdType aActorId);
+
     // If sending a sync message returns an error, this function gives a more
     // descriptive error message.
     SyncSendError LastSendError() const {

ipc/ipdl/ipdl/lower.py

@@ -514,6 +514,8 @@ class _PromiseRejectReason:
     SendError = ExprVar('PromiseRejectReason::SendError')
     ChannelClosed = ExprVar('PromiseRejectReason::ChannelClosed')
     HandlerRejected = ExprVar('PromiseRejectReason::HandlerRejected')
+    ActorDestroyed = ExprVar('PromiseRejectReason::ActorDestroyed')
+
 
 ##-----------------------------------------------------------------------------
 ## Intermediate representation (IR) nodes used during lowering
@@ -3050,6 +3052,18 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
 
         if len(ptype.manages):
             destroysubtree.addstmt(Whitespace.NL)
+
+        # Reject pending promises for actor before calling ActorDestroy().
+        rejectPendingPromiseMethod = ExprSelect(self.protocol.callGetChannel(),
+                                                '->',
+                                                'RejectPendingPromisesForActor')
+        destroysubtree.addstmts([ Whitespace('// Reject owning pending promises.\n',
+                                             indent=1),
+                                  StmtExpr(ExprCall(rejectPendingPromiseMethod,
+                                                    args=[ ExprVar('this') ])),
+                                  Whitespace.NL
+                                 ])
+
         destroysubtree.addstmts([ Whitespace('// Finally, destroy "us".\n',
                                              indent=1),
                                   StmtExpr(ExprCall(_destroyMethod(),
@@ -4253,7 +4267,15 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
         promisethen.addstmts(sendmsg)
         promiserej = ExprLambda([ExprVar.THIS, routingId, seqno],
                                 [Decl(_PromiseRejectReason.Type(), reason.name)])
-        promiserej.addstmts([ StmtExpr(ExprCall(ExprVar('MOZ_ASSERT'),
+
+        # If-statement for silently cancelling the promise due to ActorDestroyed.
+        returnifactordestroyed = StmtIf(ExprBinary(reason, '==',
+                                                   _PromiseRejectReason.ActorDestroyed))
+        returnifactordestroyed.addifstmts([_printWarningMessage("Reject due to ActorDestroyed"),
+                                           StmtReturn()])
+
+        promiserej.addstmts([ returnifactordestroyed,
+                              StmtExpr(ExprCall(ExprVar('MOZ_ASSERT'),
                                                 args=[ ExprBinary(reason, '==',
                                                                   _PromiseRejectReason.HandlerRejected) ])),
                               StmtExpr(ExprAssn(reason, _PromiseRejectReason.HandlerRejected)),
@@ -4515,7 +4537,8 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
                   self.profilerLabel(md) ] + self.transition(md, actor)
 
         if md.returns:
-            sendargs.append(ExprCall(ExprSelect(retpromise, '.', 'get')));
+            sendargs.append(ExprCall(ExprSelect(retpromise, '.', 'get')))
+            sendargs.append(ExprVar('this'))
             stmts.extend(promisedecl)
             retvar = retpromise