Bug 893172 - Call ShutDownProcess in ContentParent::ActorDestroy. r=bent
authorJustin Lebar <justin.lebar@gmail.com>
Mon, 15 Jul 2013 08:55:13 -0400
changeset 138476 fb0206edf864e9f304ead0d23494da1711749fee
parent 138475 d8a89848224bcf0ceb00fb13148d9cedf461bcce
child 138477 563fd3adc4014faf36b337d29730503efc6b96e4
push idunknown
push userunknown
push dateunknown
reviewersbent
bugs893172
milestone25.0a1
Bug 893172 - Call ShutDownProcess in ContentParent::ActorDestroy. r=bent
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -725,29 +725,38 @@ ContentParent::TransformPreallocatedInto
     MOZ_ASSERT(IsPreallocated());
     mAppManifestURL = aAppManifestURL;
     TryGetNameFromManifestURL(aAppManifestURL, mAppName);
 
     return SendSetProcessPrivileges(aPrivs);
 }
 
 void
-ContentParent::ShutDownProcess()
+ContentParent::ShutDownProcess(bool aFromActorDestroyed)
 {
   if (!mIsDestroyed) {
+    mIsDestroyed = true;
+
     const InfallibleTArray<PIndexedDBParent*>& idbParents =
       ManagedPIndexedDBParent();
     for (uint32_t i = 0; i < idbParents.Length(); ++i) {
       static_cast<IndexedDBParent*>(idbParents[i])->Disconnect();
     }
 
-    // Close() can only be called once.  It kicks off the
-    // destruction sequence.
-    Close();
-    mIsDestroyed = true;
+    if (aFromActorDestroyed) {
+      // If ActorDestroyed() is calling this method but mIsDestroyed was false,
+      // we're in an error state.
+      AsyncChannel* channel = GetIPCChannel();
+      if (channel) {
+        channel->CloseWithError();
+      }
+    } else {
+      // Close() can only be called once: It kicks off the destruction sequence.
+      Close();
+    }
   }
   // NB: must MarkAsDead() here so that this isn't accidentally
   // returned from Get*() while in the midst of shutdown.
   MarkAsDead();
 }
 
 void
 ContentParent::MarkAsDead()
@@ -943,16 +952,21 @@ ContentParent::ActorDestroy(ActorDestroy
                 nsAutoString dumpID(crashReporter->ChildDumpID());
                 props->SetPropertyAsAString(NS_LITERAL_STRING("dumpID"), dumpID);
             }
 #endif
         }
         obs->NotifyObservers((nsIPropertyBag2*) props, "ipc:content-shutdown", nullptr);
     }
 
+    // If the child process was terminated due to a SIGKIL, ShutDownProcess
+    // might not have been called yet.  We must call it to ensure that our
+    // channel is closed, etc.
+    ShutDownProcess(/* aForce = */ true);
+
     MessageLoop::current()->
         PostTask(FROM_HERE,
                  NewRunnableFunction(DelayedDeleteSubprocess, mSubprocess));
     mSubprocess = NULL;
 
     // IPDL rules require actors to live on past ActorDestroy, but it
     // may be that the kungFuDeathGrip above is the last reference to
     // |this|.  If so, when we go out of scope here, we're deleted and
@@ -1001,17 +1015,18 @@ ContentParent::NotifyTabDestroyed(PBrows
     }
 
     // There can be more than one PBrowser for a given app process
     // because of popup windows.  When the last one closes, shut
     // us down.
     if (ManagedPBrowserParent().Length() == 1) {
         MessageLoop::current()->PostTask(
             FROM_HERE,
-            NewRunnableMethod(this, &ContentParent::ShutDownProcess));
+            NewRunnableMethod(this, &ContentParent::ShutDownProcess,
+                              /* force */ false));
     }
 }
 
 jsipc::JavaScriptParent*
 ContentParent::GetCPOWManager()
 {
     if (ManagedPJavaScriptParent().Length()) {
         return static_cast<JavaScriptParent*>(ManagedPJavaScriptParent()[0]);
@@ -1459,17 +1474,17 @@ NS_IMPL_THREADSAFE_ISUPPORTS3(ContentPar
                               nsIDOMGeoPositionCallback)
 
 NS_IMETHODIMP
 ContentParent::Observe(nsISupports* aSubject,
                        const char* aTopic,
                        const PRUnichar* aData)
 {
     if (!strcmp(aTopic, "xpcom-shutdown") && mSubprocess) {
-        ShutDownProcess();
+        ShutDownProcess(/* force */ false);
         NS_ASSERTION(!mSubprocess, "Close should have nulled mSubprocess");
     }
 
     if (!mIsAlive || !mSubprocess)
         return NS_OK;
 
     // listening for memory pressure event
     if (!strcmp(aTopic, "memory-pressure") &&
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -237,18 +237,24 @@ private:
      */
     void MarkAsDead();
 
     /**
      * Exit the subprocess and vamoose.  After this call IsAlive()
      * will return false and this ContentParent will not be returned
      * by the Get*() funtions.  However, the shutdown sequence itself
      * may be asynchronous.
+     *
+     * If aFromActorDestroyed is true and this is the first call to
+     * ShutDownProcess, then we'll close our channel using CloseWithError()
+     * rather than vanilla Close().  CloseWithError() indicates to IPC that this
+     * is an abnormal shutdown (e.g. a crash); when the process shuts down
+     * cleanly, ShutDownProcess runs before ActorDestroyed.
      */
-    void ShutDownProcess();
+    void ShutDownProcess(bool aFromActorDestroyed);
 
     PCompositorParent*
     AllocPCompositorParent(mozilla::ipc::Transport* aTransport,
                            base::ProcessId aOtherProcess) MOZ_OVERRIDE;
     PImageBridgeParent*
     AllocPImageBridgeParent(mozilla::ipc::Transport* aTransport,
                             base::ProcessId aOtherProcess) MOZ_OVERRIDE;