Bug 1319910 - Crash child, not parent, on FatalError in TestActorPunning and TestBadActor. r=billm draft
authorAndrew McCreight <continuation@gmail.com>
Wed, 23 Nov 2016 13:40:04 -0800
changeset 443124 db64e914baa667b8204886f4e1ad03f147807498
parent 443123 8e81ae8c19e987b97019ee061290e7686c35dd21
child 537973 1e3618cb2dbebdb1a9092b6a32810fcb201014ed
push id36908
push userbmo:continuation@gmail.com
push dateWed, 23 Nov 2016 22:24:50 +0000
reviewersbillm
bugs1319910
milestone53.0a1
Bug 1319910 - Crash child, not parent, on FatalError in TestActorPunning and TestBadActor. r=billm The parent process crashes if it gets a bad message from the child, but that makes it hard to test. This patch overrides the fatal error handling method and uses the old behavior, that kills the child. I copied the code to kill the child from TestHangs. MozReview-Commit-ID: 3YgqaCgHGI0
ipc/ipdl/test/cxx/TestActorPunning.cpp
ipc/ipdl/test/cxx/TestActorPunning.h
ipc/ipdl/test/cxx/TestBadActor.cpp
ipc/ipdl/test/cxx/TestBadActor.h
--- a/ipc/ipdl/test/cxx/TestActorPunning.cpp
+++ b/ipc/ipdl/test/cxx/TestActorPunning.cpp
@@ -20,16 +20,40 @@ mozilla::ipc::IPCResult
 TestActorPunningParent::RecvPun(PTestActorPunningSubParent* a, const Bad& bad)
 {
     if (a->SendBad())
         fail("bad!");
     fail("shouldn't have received this message in the first place");
     return IPC_OK();
 }
 
+// By default, fatal errors kill the parent process, but this makes it
+// hard to test, so instead we use the previous behavior and kill the
+// child process.
+void
+TestActorPunningParent::HandleFatalError(const char* aProtocolName, const char* aErrorMsg) const
+{
+  if (!!strcmp(aProtocolName, "PTestActorPunningParent")) {
+    fail("wrong protocol hit a fatal error");
+  }
+
+  if (!!strcmp(aErrorMsg, "Error deserializing 'PTestActorPunningSubParent'")) {
+    fail("wrong fatal error");
+  }
+
+  ipc::ScopedProcessHandle otherProcessHandle;
+  if (!base::OpenProcessHandle(OtherPid(), &otherProcessHandle.rwget())) {
+    fail("couldn't open child process");
+  } else {
+    if (!base::KillProcess(otherProcessHandle, 0, false)) {
+      fail("terminating child process");
+    }
+  }
+}
+
 PTestActorPunningPunnedParent*
 TestActorPunningParent::AllocPTestActorPunningPunnedParent()
 {
     return new TestActorPunningPunnedParent();
 }
 
 bool
 TestActorPunningParent::DeallocPTestActorPunningPunnedParent(PTestActorPunningPunnedParent* a)
--- a/ipc/ipdl/test/cxx/TestActorPunning.h
+++ b/ipc/ipdl/test/cxx/TestActorPunning.h
@@ -30,20 +30,22 @@ protected:
     PTestActorPunningSubParent* AllocPTestActorPunningSubParent() override;
     bool DeallocPTestActorPunningSubParent(PTestActorPunningSubParent* a) override;
 
     virtual mozilla::ipc::IPCResult RecvPun(PTestActorPunningSubParent* a, const Bad& bad) override;
 
     virtual void ActorDestroy(ActorDestroyReason why) override
     {
         if (NormalShutdown == why)
-            fail("should have died from error!");  
+            fail("should have died from error!");
         passed("ok");
         QuitParent();
     }
+
+    virtual void HandleFatalError(const char* aProtocolName, const char* aErrorMsg) const override;
 };
 
 class TestActorPunningPunnedParent :
     public PTestActorPunningPunnedParent
 {
 public:
     TestActorPunningPunnedParent() {}
     virtual ~TestActorPunningPunnedParent() {}
--- a/ipc/ipdl/test/cxx/TestBadActor.cpp
+++ b/ipc/ipdl/test/cxx/TestBadActor.cpp
@@ -14,16 +14,40 @@ TestBadActorParent::Main()
 
   PTestBadActorSubParent* child = SendPTestBadActorSubConstructor();
   if (!child)
     fail("Sending constructor");
 
   Unused << child->Call__delete__(child);
 }
 
+// By default, fatal errors kill the parent process, but this makes it
+// hard to test, so instead we use the previous behavior and kill the
+// child process.
+void
+TestBadActorParent::HandleFatalError(const char* aProtocolName, const char* aErrorMsg) const
+{
+  if (!!strcmp(aProtocolName, "PTestBadActorSubParent")) {
+    fail("wrong protocol hit a fatal error");
+  }
+
+  if (!!strcmp(aErrorMsg, "incoming message racing with actor deletion")) {
+    fail("wrong fatal error");
+  }
+
+  ipc::ScopedProcessHandle otherProcessHandle;
+  if (!base::OpenProcessHandle(OtherPid(), &otherProcessHandle.rwget())) {
+    fail("couldn't open child process");
+  } else {
+    if (!base::KillProcess(otherProcessHandle, 0, false)) {
+      fail("terminating child process");
+    }
+  }
+}
+
 PTestBadActorSubParent*
 TestBadActorParent::AllocPTestBadActorSubParent()
 {
   return new TestBadActorSubParent();
 }
 
 mozilla::ipc::IPCResult
 TestBadActorSubParent::RecvPing()
--- a/ipc/ipdl/test/cxx/TestBadActor.h
+++ b/ipc/ipdl/test/cxx/TestBadActor.h
@@ -28,16 +28,18 @@ protected:
   virtual void ActorDestroy(ActorDestroyReason why) override
   {
     if (AbnormalShutdown != why)
       fail("unexpected destruction");
     passed("ok");
     QuitParent();
   }
 
+  virtual void HandleFatalError(const char* aProtocolName, const char* aErrorMsg) const override;
+
   virtual PTestBadActorSubParent*
   AllocPTestBadActorSubParent() override;
 
   virtual bool
   DeallocPTestBadActorSubParent(PTestBadActorSubParent* actor) override {
     delete actor;
     return true;
   }