Bug 546035: Test

ipc/ipdl/test/cxx/PTestHangs.ipdl

@@ -6,13 +6,25 @@ rpc protocol PTestHangs {
 both:
     rpc StackFrame();
 
+parent:
+    async Nonce();
+
 child:
+    async Start();
     rpc Hang();
     __delete__();
 
 
 state START:
+    send Start goto RACE;
+
+state RACE:
+    recv Nonce goto RACE1;
+    call StackFrame goto RACE2;
+state RACE1:
     call StackFrame goto FRAME2;
+state RACE2:
+    recv Nonce goto FRAME2;
 
 // So as to test unwinding the RPC stack
 state FRAME2: answer StackFrame goto FRAME3;

ipc/ipdl/test/cxx/TestHangs.cpp

@@ -6,8 +6,12 @@
 
 using base::KillProcess;
 
-// XXX could drop this; very conservative
-static const int kTimeoutSecs = 5;
+template<>
+struct RunnableMethodTraits<mozilla::_ipdltest::TestHangsParent>
+{
+    static void RetainCallee(mozilla::_ipdltest::TestHangsParent* obj) { }
+    static void ReleaseCallee(mozilla::_ipdltest::TestHangsParent* obj) { }
+};
 
 namespace mozilla {
 namespace _ipdltest {
@@ -15,7 +19,7 @@ namespace _ipdltest {
 //-----------------------------------------------------------------------------
 // parent
 
-TestHangsParent::TestHangsParent() : mFramesToGo(2)
+TestHangsParent::TestHangsParent() : mFramesToGo(2), mDetectedHang(false)
 {
     MOZ_COUNT_CTOR(TestHangsParent);
 }
@@ -28,27 +32,57 @@ TestHangsParent::~TestHangsParent()
 void
 TestHangsParent::Main()
 {
-    SetReplyTimeoutMs(1000 * kTimeoutSecs);
+    // Here we try to set things up to test the following sequence of events:
+    //
+    // - subprocess causes an OnMaybeDequeueOne() task to be posted to
+    //   this thread
+    //
+    // - subprocess hangs just long enough for the hang timer to expire
+    //
+    // - hang-kill code in the parent starts running
+    //
+    // - subprocess replies to message while hang code runs
+    //
+    // - reply is processed in OnMaybeDequeueOne() before Close() has
+    //   been called or the channel error notification has been posted
 
-    if (CallStackFrame())
+    // this tells the subprocess to send us Nonce()
+    if (!SendStart())
+        fail("sending Start");
+
+    // now we sleep here for a while awaiting the Nonce() message from
+    // the child.  since we're not blocked on anything, the IO thread
+    // will enqueue an OnMaybeDequeueOne() task to process that
+    // message
+    // 
+    // NB: PR_Sleep is exactly what we want, only the current thread
+    // sleeping
+    PR_Sleep(5000);
+
+    // when we call into this, we'll pull the Nonce() message out of
+    // the mPending queue, but that doesn't matter ... the
+    // OnMaybeDequeueOne() event will remain
+    if (CallStackFrame() && mDetectedHang)
         fail("should have timed out!");
 
-    Close();
+    // the Close() task in the queue will shut us down
 }
 
 bool
 TestHangsParent::ShouldContinueFromReplyTimeout()
 {
-    // If we kill the subprocess here, then the "channel error" event
-    // posted by the IO thread will race with the |Close()| above, in
-    // |Main()|.  As killing the child process will probably be a
-    // common action to take from ShouldContinue(), we need to ensure
-    // that *Channel can deal.
+    mDetectedHang = true;
 
-    // XXX: OtherProcess() is a semi-private API, but using it is
-    // OK until we start worrying about inter-thread comm
-    if (!KillProcess(OtherProcess(), 0, false))
-        fail("terminating child process");
+    // so we've detected a timeout after 1 ms ... now we cheat and
+    // sleep for a long time, to allow the subprocess's reply to come
+    // in
+
+    PR_Sleep(5000);
+
+    // reply should be here; we'll post a task to shut things down.
+    // This must be after OnMaybeDequeueOne() in the event queue.
+    MessageLoop::current()->PostTask(
+        FROM_HERE, NewRunnableMethod(this, &TestHangsParent::CleanUp));
 
     return false;
 }
@@ -61,6 +95,10 @@ TestHangsParent::AnswerStackFrame()
             fail("should have timed out!");
     }
     else {
+        // minimum possible, 1 ms.  We want to detecting a hang to race
+        // with the reply coming in, as reliably as possible
+        SetReplyTimeoutMs(1);
+
         if (CallHang())
             fail("should have timed out!");
     }
@@ -68,6 +106,14 @@ TestHangsParent::AnswerStackFrame()
     return true;
 }
 
+void
+TestHangsParent::CleanUp()
+{
+    if (!KillProcess(OtherProcess(), 0, false))
+        fail("terminating child process");
+    Close();
+}
+
 
 //-----------------------------------------------------------------------------
 // child
@@ -85,14 +131,14 @@ TestHangsChild::~TestHangsChild()
 bool
 TestHangsChild::AnswerHang()
 {
-    puts(" (child process is hanging now)");
+    puts(" (child process is 'hanging' now)");
 
-    // XXX: pause() is right for this, but windows doesn't appear to
-    // implement it.  So sleep for 100,000 seconds instead.
-    PR_Sleep(PR_SecondsToInterval(100000));
+    // just sleep until we're reasonably confident the 1ms hang
+    // detector fired in the parent process and it's sleeping in
+    // ShouldContinueFromReplyTimeout()
+    PR_Sleep(1000);
 
-    fail("should have been killed!");
-    return false;               // not reached
+    return true;
 }
 
 } // namespace _ipdltest

ipc/ipdl/test/cxx/TestHangs.h

@@ -23,6 +23,11 @@ protected:
     NS_OVERRIDE
     virtual bool ShouldContinueFromReplyTimeout();
 
+    NS_OVERRIDE
+    virtual bool RecvNonce() {
+        return true;
+    }
+
     NS_OVERRIDE
     virtual bool AnswerStackFrame();
 
@@ -35,8 +40,11 @@ protected:
         QuitParent();
     }
 
+    void CleanUp();
+
     // XXX hack around lack of State()
     int mFramesToGo;
+    bool mDetectedHang;
 };
 
 
@@ -48,11 +56,18 @@ public:
     virtual ~TestHangsChild();
 
 protected:
+    NS_OVERRIDE
+    virtual bool RecvStart() {
+        if (!SendNonce())
+            fail("sending Nonce");
+        return true;
+    }
+
     NS_OVERRIDE
     virtual bool AnswerStackFrame()
     {
-        if (!CallStackFrame())
-            fail("shouldn't be able to observe this failure");
+        if (CallStackFrame())
+            fail("should have failed");
         return true;
     }
 
@@ -62,7 +77,9 @@ protected:
     NS_OVERRIDE
     virtual void ActorDestroy(ActorDestroyReason why)
     {
-        fail("should have been mercilessly killed");
+        if (AbnormalShutdown != why)
+            fail("unexpected destruction!");
+        QuitChild();
     }
 };