Bug 525181, part 3: Turn on "soft" state checking, NS_WARNING()ing on bad transitions. Fix tests. r=bent

2024-11-29 15:52:07 +00:00 · 2010-07-15 14:27:43 -05:00 · 2010-07-15 14:27:43 -05:00 · c6a03599aa
commit c6a03599aa
parent e2e5908a4e
10 changed files with 95 additions and 45 deletions
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@ -361,6 +361,12 @@ def _printErrorMessage(msg):
    return StmtExpr(
        ExprCall(ExprVar('NS_ERROR'), args=[ msg ]))

+def _printWarningMessage(msg):
+    if isinstance(msg, str):
+        msg = ExprLiteral.String(msg)
+    return StmtExpr(
+        ExprCall(ExprVar('NS_WARNING'), args=[ msg ]))
+
 def _fatalError(msg):
    return StmtExpr(
        ExprCall(ExprVar('FatalError'), args=[ ExprLiteral.String(msg) ]))
@ -373,6 +379,10 @@ def _killProcess(pid):
               ExprVar('base::PROCESS_END_KILLED_BY_USER'),
               ExprLiteral.FALSE ])

+def _badTransition():
+    # FIXME: make this a FatalError()
+    return [ _printWarningMessage('bad state transition!') ]
+
 # Results that IPDL-generated code returns back to *Channel code.
 # Users never see these
 class _Result:
@ -2222,7 +2232,8 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
            Typedef(Type(self.protocol.channelName()), 'Channel'),
            Typedef(Type(self.protocol.fqListenerName()), 'ChannelListener'),
            Typedef(Type('base::ProcessHandle'), 'ProcessHandle'),
-            Typedef(Type('mozilla::ipc::SharedMemory'), 'SharedMemory')
+            Typedef(Type('mozilla::ipc::SharedMemory'), 'SharedMemory'),
+            Typedef(Type('mozilla::ipc::Trigger'), 'Trigger')
        ]


@ -4218,6 +4229,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
        idvar, saveIdStmts = self.saveActorId(md)
        case.addstmts(
            stmts
+            + self.transition(md, 'in')
            + [ StmtDecl(Decl(r.bareType(self.side), r.var().name))
                for r in md.returns ]
            # alloc the actor, register it under the foreign ID
@ -4245,6 +4257,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
        idvar, saveIdStmts = self.saveActorId(md)
        case.addstmts(
            stmts
+            + self.transition(md, 'in')
            + [ StmtDecl(Decl(r.bareType(self.side), r.var().name))
                for r in md.returns ]
            + self.invokeRecvHandler(md, implicit=0)
@ -4268,6 +4281,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
        idvar, saveIdStmts = self.saveActorId(md)
        case.addstmts(
            stmts
+            + self.transition(md, 'in')
            + [ StmtDecl(Decl(r.bareType(self.side), r.var().name))
                for r in md.returns ]
            + saveIdStmts
@ -4420,30 +4434,34 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
        sendok = ExprVar('__sendok')
        return (
            sendok,
-            [ Whitespace.NL,
-              self.logMessage(md, msgexpr, 'Sending '),
-              Whitespace.NL,
-              StmtDecl(Decl(Type.BOOL, sendok.name),
-                       init=ExprCall(
-                           ExprSelect(self.protocol.channelVar(actor),
-                                      self.protocol.channelSel(), 'Send'),
-                           args=[ msgexpr ]))
+            ([ Whitespace.NL,
+               self.logMessage(md, msgexpr, 'Sending ') ]
+            + self.transition(md, 'out', actor)
+            + [ Whitespace.NL,
+                StmtDecl(Decl(Type.BOOL, sendok.name),
+                         init=ExprCall(
+                             ExprSelect(self.protocol.channelVar(actor),
+                                        self.protocol.channelSel(), 'Send'),
+                             args=[ msgexpr ]))
            ])
+        )

    def sendBlocking(self, md, msgexpr, replyexpr, actor=None):
        sendok = ExprVar('__sendok')
        return (
            sendok,
-            [ Whitespace.NL,
-              self.logMessage(md, msgexpr, 'Sending '),
-              Whitespace.NL,
-              StmtDecl(
-                  Decl(Type.BOOL, sendok.name),
-                  init=ExprCall(ExprSelect(self.protocol.channelVar(actor),
-                                           self.protocol.channelSel(),
-                                           _sendPrefix(md.decl.type)),
-                                args=[ msgexpr, ExprAddrOf(replyexpr) ]))
+            ([ Whitespace.NL,
+               self.logMessage(md, msgexpr, 'Sending ') ]
+            + self.transition(md, 'out', actor)
+            + [ Whitespace.NL,
+                StmtDecl(
+                    Decl(Type.BOOL, sendok.name),
+                    init=ExprCall(ExprSelect(self.protocol.channelVar(actor),
+                                             self.protocol.channelSel(),
+                                             _sendPrefix(md.decl.type)),
+                                  args=[ msgexpr, ExprAddrOf(replyexpr) ]))
            ])
+        )

    def callAllocActor(self, md, retsems):
        return ExprCall(
@ -4517,6 +4535,28 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
            saveIdStmts = [ ]
        return idvar, saveIdStmts

+    def transition(self, md, direction, actor=None):
+        if actor is not None:  stateexpr = _actorState(actor)
+        else:                  stateexpr = self.protocol.stateVar()
+        
+        if (self.side is 'parent' and direction is 'out'
+            or self.side is 'child' and direction is 'in'):
+            action = ExprVar('Trigger::Send')
+        elif (self.side is 'parent' and direction is 'in'
+            or self.side is 'child' and direction is 'out'):
+            action = ExprVar('Trigger::Recv')
+        else: assert 0 and 'unknown combo %s/%s'% (self.side, direction)
+
+        ifbad = StmtIf(ExprNot(
+            ExprCall(
+                ExprVar(self.protocol.name +'::Transition'),
+                args=[ stateexpr,
+                       ExprCall(ExprVar('Trigger'),
+                                args=[ action, ExprVar(md.pqMsgId()) ]),
+                       ExprAddrOf(stateexpr) ])))
+        ifbad.addifstmts(_badTransition())
+        return [ ifbad ]
+
    def checkedRead(self, ipdltype, expr, msgexpr, iterexpr, errfn):
        ifbad = StmtIf(ExprNot(self.read(ipdltype, expr, msgexpr, iterexpr)))
        ifbad.addifstmts(errfn('error deserializing (better message TODO)'))
--- a/ipc/ipdl/test/cxx/PTestDataStructures.ipdl
+++ b/ipc/ipdl/test/cxx/PTestDataStructures.ipdl
@ -188,9 +188,9 @@ parent:
    // (nsIntRegion isn't memmove()able)
    sync Test18(nsIntRegion[] ops);

-state START:
+state CONSTRUCTING:
+    send PTestDataStructuresSub goto CONSTRUCTING;
    send Start goto TEST1;
-
 state TEST1:   recv Test1 goto TEST2;
 state TEST2:   recv Test2 goto TEST3;
 state TEST3:   recv Test3 goto TEST4;
--- a/ipc/ipdl/test/cxx/PTestDesc.ipdl
+++ b/ipc/ipdl/test/cxx/PTestDesc.ipdl
@ -11,16 +11,20 @@ child:

    Test(PTestDescSubsub a);

+    __delete__();
+
 parent:
    Ok(PTestDescSubsub a);


-state START:
+state CONSTRUCT:
+    call PTestDescSub goto TEST;
+state TEST:
    send Test goto ACK;
-
 state ACK:
-    recv Ok goto ACK;
-    // delete
+    recv Ok goto DEAD;
+state DEAD:
+    send __delete__;
 };

 }
--- a/ipc/ipdl/test/cxx/PTestRPCRaces.ipdl
+++ b/ipc/ipdl/test/cxx/PTestRPCRaces.ipdl
@ -59,11 +59,20 @@ state RACE3:
    call Child goto DUMMY3_1;
    answer Parent goto DUMMY3_2;
 state DUMMY3_1:
+    // the parent receives this from the child in this state
+    recv GetAnsweredParent goto CHECK;
+    // this transition is never taken (if the custom race resolution
+    // works correctly)
    answer Parent goto CHECK;
 state DUMMY3_2:
    call Child goto CHECK;
 state CHECK:
+    // the child sends this from this state
    recv GetAnsweredParent goto DYING;
+    // because of deferred processing, the parent receives the child's
+    // message here
+    answer Parent goto DYING;
+

 state DYING:
    send __delete__;
--- a/ipc/ipdl/test/cxx/PTestSelfManage.ipdl
+++ b/ipc/ipdl/test/cxx/PTestSelfManage.ipdl
@ -13,9 +13,7 @@ child:
    __delete__();

 state LIVE:
-    send PTestSelfManage goto DEAD;
-
-state DEAD:
+    send PTestSelfManage goto LIVE;
    send __delete__;
 };

--- a/ipc/ipdl/test/cxx/TestHangs.cpp
+++ b/ipc/ipdl/test/cxx/TestHangs.cpp
@ -19,7 +19,7 @@ namespace _ipdltest {
 //-----------------------------------------------------------------------------
 // parent

-TestHangsParent::TestHangsParent() : mFramesToGo(2), mDetectedHang(false)
+TestHangsParent::TestHangsParent() : mDetectedHang(false)
 {
    MOZ_COUNT_CTOR(TestHangsParent);
 }
@ -90,7 +90,7 @@ TestHangsParent::ShouldContinueFromReplyTimeout()
 bool
 TestHangsParent::AnswerStackFrame()
 {
-    if (--mFramesToGo) {
+    if (PTestHangs::HANG != state()) {
        if (CallStackFrame())
            fail("should have timed out!");
    }
--- a/ipc/ipdl/test/cxx/TestHangs.h
+++ b/ipc/ipdl/test/cxx/TestHangs.h
@ -42,8 +42,6 @@ protected:

    void CleanUp();

-    // XXX hack around lack of State()
-    int mFramesToGo;
    bool mDetectedHang;
 };

--- a/ipc/ipdl/test/cxx/TestLatency.cpp
+++ b/ipc/ipdl/test/cxx/TestLatency.cpp
@ -18,8 +18,7 @@ TestLatencyParent::TestLatencyParent() :
    mPP5TimeTotal(),
    mRpcTimeTotal(),
    mPPTrialsToGo(NR_TRIALS),
-    mPP5TrialsToGo(NR_TRIALS),
-    mPongsToGo(0)
+    mPP5TrialsToGo(NR_TRIALS)
 {
    MOZ_COUNT_CTOR(TestLatencyParent);
 }
@ -60,8 +59,6 @@ void
 TestLatencyParent::Ping5Pong5Trial()
 {
    mStart = TimeStamp::Now();
-    // HACK
-    mPongsToGo = 5;

    if (!SendPing5() ||
        !SendPing5() ||
@ -97,8 +94,7 @@ TestLatencyParent::RecvPong()
 bool
 TestLatencyParent::RecvPong5()
 {
-    // HACK
-    if (0 < --mPongsToGo)
+    if (PTestLatency::PING5 != state())
        return true;

    TimeDuration thisTrial = (TimeStamp::Now() - mStart);
@ -159,7 +155,16 @@ TestLatencyChild::RecvPing()
 bool
 TestLatencyChild::RecvPing5()
 {
-    SendPong5();
+    if (PTestLatency::PONG1 != state())
+        return true;
+
+    if (!SendPong5() ||
+        !SendPong5() ||
+        !SendPong5() ||
+        !SendPong5() ||
+        !SendPong5())
+        fail("sending Pong5()");
+
    return true;
 }

--- a/ipc/ipdl/test/cxx/TestLatency.h
+++ b/ipc/ipdl/test/cxx/TestLatency.h
@ -61,9 +61,6 @@ private:

    int mPPTrialsToGo;
    int mPP5TrialsToGo;
-
-    // FIXME/cjones: HACK ALERT: don't need this once IPDL exposes actor state
-    int mPongsToGo;
 };


--- a/ipc/ipdl/test/cxx/TestStackHooks.cpp
+++ b/ipc/ipdl/test/cxx/TestStackHooks.cpp
@ -96,17 +96,16 @@ TestStackHooksChild::AnswerStackFrame()
    if (1 != mIncallDepth)
        fail("missed EnteredCall or ExitedCall hook");

-    // FIXME use IPDL state instead
-    if (4 == mEntered) {        // test 4
+    if (PTestStackHooks::TEST4_3 == state()) {
        if (!SendAsync())
            fail("sending Async()");
    }
-    else if (5 == mEntered) {   // test 5
+    else if (PTestStackHooks::TEST5_3 == state()) {
        if (!SendSync())
            fail("sending Sync()");
    }
    else {
-        fail("unexpected |mEntered| count");
+        fail("unexpected state");
    }

    if (!mOnStack)