b=550026 Cancel PluginCrashed when PluginModule is deleted. r=bsmedberg

dom/plugins/PluginModuleParent.cpp

@@ -70,24 +70,6 @@ struct RunnableMethodTraits<mozilla::plugins::PluginModuleParent>
     static void ReleaseCallee(Class* obj) { }
 };
 
-class PluginCrashed : public nsRunnable
-{
-public:
-    PluginCrashed(nsNPAPIPlugin* plugin,
-                  const nsString& dumpID)
-        : mDumpID(dumpID),
-          mPlugin(plugin) { }
-
-    NS_IMETHOD Run() {
-        mPlugin->PluginCrashed(mDumpID);
-        return NS_OK;
-    }
-
-private:
-    nsNPAPIPlugin* mPlugin;
-    nsString mDumpID;
-};
-
 // static
 PluginLibrary*
 PluginModuleParent::LoadModule(const char* aFilePath)
@@ -112,6 +94,7 @@ PluginModuleParent::PluginModuleParent(const char* aFilePath)
     , mNPNIface(NULL)
     , mPlugin(NULL)
     , mProcessStartTime(time(NULL))
+    , mPluginCrashedTask(NULL)
 {
     NS_ASSERTION(mSubprocess, "Out of memory!");
 
@@ -124,6 +107,13 @@ PluginModuleParent::PluginModuleParent(const char* aFilePath)
 
 PluginModuleParent::~PluginModuleParent()
 {
+    NS_ASSERTION(OkToCleanup(), "unsafe destruction");
+
+    if (mPluginCrashedTask) {
+        mPluginCrashedTask->Cancel();
+        mPluginCrashedTask = 0;
+    }
+
     if (!mShutdown) {
         NS_WARNING("Plugin host deleted the module without shutting down.");
         NPError err;
@@ -269,12 +259,11 @@ PluginModuleParent::ActorDestroy(ActorDestroyReason why)
     switch (why) {
     case AbnormalShutdown: {
         nsCOMPtr<nsIFile> dump;
-        nsAutoString dumpID;
         if (GetMinidump(getter_AddRefs(dump))) {
             WriteExtraDataForMinidump(dump);
-            if (NS_SUCCEEDED(dump->GetLeafName(dumpID))) {
-                dumpID.Replace(dumpID.Length() - 4, 4,
-                               NS_LITERAL_STRING(""));
+            if (NS_SUCCEEDED(dump->GetLeafName(mDumpID))) {
+                mDumpID.Replace(mDumpID.Length() - 4, 4,
+                                NS_LITERAL_STRING(""));
             }
         }
         else {
@@ -285,9 +274,9 @@ PluginModuleParent::ActorDestroy(ActorDestroyReason why)
         // Defer the PluginCrashed method so that we don't re-enter
         // and potentially modify the actor child list while enumerating it.
         if (mPlugin) {
-            nsCOMPtr<nsIRunnable> r =
-                new PluginCrashed(mPlugin, dumpID);
-            NS_DispatchToMainThread(r);
+            mPluginCrashedTask = NewRunnableMethod(
+                this, &PluginModuleParent::NotifyPluginCrashed);
+            MessageLoop::current()->PostTask(FROM_HERE, mPluginCrashedTask);
         }
         break;
     }
@@ -300,6 +289,16 @@ PluginModuleParent::ActorDestroy(ActorDestroyReason why)
     }
 }
 
+void
+PluginModuleParent::NotifyPluginCrashed()
+{
+    // MessageLoop owns this
+    mPluginCrashedTask = NULL;
+
+    if (mPlugin)
+        mPlugin->PluginCrashed(mDumpID);
+}
+
 PPluginInstanceParent*
 PluginModuleParent::AllocPPluginInstance(const nsCString& aMimeType,
                                          const uint16_t& aMode,

dom/plugins/PluginModuleParent.h

@@ -120,6 +120,10 @@ public:
 
     bool EnsureValidNPIdentifier(NPIdentifier aIdentifier);
 
+    bool OkToCleanup() const {
+        return !IsOnCxxStack();
+    }
+
 protected:
     NS_OVERRIDE
     virtual mozilla::ipc::RPCChannel::RacyRPCPolicy
@@ -235,6 +239,7 @@ private:
                              const char* value);
     void CleanupFromTimeout();
     static int TimeoutChanged(const char* aPref, void* aModule);
+    void NotifyPluginCrashed();
 
     nsCString mCrashNotes;
     PluginProcessParent* mSubprocess;
@@ -243,6 +248,8 @@ private:
     nsTHashtable<nsVoidPtrHashKey> mValidIdentifiers;
     nsNPAPIPlugin* mPlugin;
     time_t mProcessStartTime;
+    CancelableTask* mPluginCrashedTask;
+    nsString mDumpID;
 };
 
 } // namespace plugins

ipc/glue/RPCChannel.cpp

@@ -181,6 +181,16 @@ RPCChannel::Call(Message* msg, Message* reply)
         NewRunnableMethod(this, &RPCChannel::OnSend, msg));
 
     while (1) {
+        // if a handler invoked by *Dispatch*() spun a nested event
+        // loop, and the connection was broken during that loop, we
+        // might have already processed the OnError event. if so,
+        // trying another loop iteration will be futile because
+        // channel state will have been cleared
+        if (!Connected()) {
+            ReportConnectionError("RPCChannel");
+            return false;
+        }
+
         // now might be the time to process a message deferred because
         // of race resolution
         MaybeProcessDeferredIncall();

ipc/glue/RPCChannel.h

@@ -138,6 +138,11 @@ public:
     // Return true iff successful.
     bool UnblockChild();
 
+    // Return true iff this has code on the C++ stack.
+    bool IsOnCxxStack() const {
+        return 0 < mCxxStackFrames;
+    }
+
     NS_OVERRIDE
     virtual bool OnSpecialMessage(uint16 id, const Message& msg);
 

ipc/ipdl/ipdl/lower.py

@@ -1356,6 +1356,10 @@ class Protocol(ipdl.ast.Protocol):
         assert self.decl.type.isToplevel()
         return ExprVar('ExitedCxxStack')
 
+    def onCxxStackVar(self):
+        assert self.decl.type.isToplevel()
+        return ExprVar('IsOnCxxStack')
+
     def nextActorIdExpr(self, side):
         assert self.decl.type.isToplevel()
         if side is 'parent':   op = '++'
@@ -2876,15 +2880,23 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
 
             self.cls.addstmts([ ontimeout, Whitespace.NL ])
 
-        # On[Entered/Exited]CxxStack()
+        # C++-stack-related methods
         if ptype.isToplevel() and toplevel.talksRpc():
+            # OnEnteredCxxStack()
             onentered = MethodDefn(MethodDecl('OnEnteredCxxStack'))
             onentered.addstmt(StmtReturn(ExprCall(p.enteredCxxStackVar())))
 
+            # OnExitedCxxStack()
             onexited = MethodDefn(MethodDecl('OnExitedCxxStack'))
             onexited.addstmt(StmtReturn(ExprCall(p.exitedCxxStackVar())))
 
-            self.cls.addstmts([ onentered, onexited, Whitespace.NL ])
+            # bool IsOnCxxStack()
+            onstack = MethodDefn(
+                MethodDecl(p.onCxxStackVar().name, ret=Type.BOOL, const=1))
+            onstack.addstmt(StmtReturn(ExprCall(
+                ExprSelect(p.channelVar(), '.', p.onCxxStackVar().name))))
+
+            self.cls.addstmts([ onentered, onexited, onstack, Whitespace.NL ])
 
         # OnChannelClose()
         onclose = MethodDefn(MethodDecl('OnChannelClose'))