RE: ISSUE-827: bug in detecting final states [SCXML-LC-Comments]

Just for the record,  here's what I think the last lines of enterStates should look like.  This replaces the existing code starting with if isFinalState(s).  Either we set running to false or we raise the done event.  If we make this change, then we do need to change the main event loop to check 'running' explicitly once we break out of the 'while running and not stable' loop.

        if isFinalState(s):
            if isSCXMLElement(s.parent):
                running = false
            else:
                parent = s.parent
                grandparent = parent.parent
                internalQueue.enqueue(new Event("done.state." + parent.id, s.donedata))
                if isParallelState(grandparent):
                    if getChildStates(grandparent).every(isInFinalState):
                        internalQueue.enqueue(new Event("done.state." + grandparent.id))

-----Original Message-----
From: Jim Barnett 
Sent: Tuesday, January 29, 2013 1:10 PM
To: Jim Barnett; Johan Roxendal
Cc: Voice Browser Working Group; www-voice@w3.org
Subject: RE: ISSUE-827: bug in detecting final states [SCXML-LC-Comments]

OK, Johan is right.  The current algorithm does work correctly.  But only because we have off-setting bugs.  The following code from enterStates is incorrect, in that we are only supposed to generate a done.state event if the parent of <final> is a state.  But this code doesn't check, so it looks to me like it will generate the done.state event when we enter a top-level <final> state. 

       if isFinalState(s):
            parent = s.parent
            grandparent = parent.parent
            internalQueue.enqueue(new Event("done.state." + parent.id, s.donedata))

Given this extra 'done' event, the algorithm will work.  But I think that we need to fix it.  (Note that 'grandparent' will be Null in the code above, because <scxml> doesn't  have a parent.  That's another problem.)

-----Original Message-----
From: Jim Barnett [mailto:Jim.Barnett@genesyslab.com] 
Sent: Tuesday, January 29, 2013 12:57 PM
To: Johan Roxendal
Cc: Voice Browser Working Group; www-voice@w3.org
Subject: RE: ISSUE-827: bug in detecting final states [SCXML-LC-Comments]

Johan,
  What internal event is left in the queue?  Is it a 'done' event generated by the entry into <final>?  The current spec requires that the 'done' event be raised on entering <final> elements that are child of <state> or <parallel>, but not <scxml>.  

- Jim

-----Original Message-----
From: Johan Roxendal [mailto:johan@roxendal.com] 
Sent: Tuesday, January 29, 2013 12:45 PM
To: Jim Barnett
Cc: Voice Browser Working Group; www-voice@w3.org
Subject: Re: ISSUE-827: bug in detecting final states [SCXML-LC-Comments]

I see the issue. 


> 
>  But the question now is:  does pyscxml implement the algorithm as defined in the latest draft?  

yes.

> It looks to me like the algorithm would:
> 1.  Take the transition triggered by e, entering a final state and setting 'running' to false.
yes.
> 2.  break out of the 'while running and not stable' loop
yes. note however that it does this _without_ removing the internal event from the queue.  
> 3.  check/execute any invokes
yes.
> 4.  check that the internal queue is empty (and hence not execute the 'continue')
not empty. the internalQueue.dequeue() call happens later in the 'while running and not stable' loop. so that part is never run when running is false. 

now, there's an argument to be made for such crucial behavior being made explicit rather than implicit in the algorithm. if we can simplify by refractoring that'd be great, otherwise some prose might be in order. 


 - Johan



> 
> -----Original Message-----
> From: Johan Roxendal [mailto:johan@roxendal.com] 
> Sent: Tuesday, January 29, 2013 12:18 PM
> To: Jim Barnett
> Cc: Voice Browser Working Group; www-voice@w3.org
> Subject: Re: ISSUE-827: bug in detecting final states [SCXML-LC-Comments]
> 
> I've run this document in pyscxml:
> 
> <scxml>
>    <state>
>        <onentry>
>            <raise event="e" />
>        </onentry>
>        <transition event="e" target="f" />
>    </state>
>    <final id="f" />
> </scxml>
> 
> from my understanding it complies with Jim's suspicions but it runs fine. of course, following the raise with a send will affect nothing as that external event will be swallowed as the machine shuts down. I see no issue here, unless I misunderstood the Jim's description. 
> 
> / Johan
> 
> 
> On Jan 29, 2013, at 16:50 , Jim Barnett <Jim.Barnett@genesyslab.com> wrote:
> 
>> Johan points out that the initial example does not really illustrate the bug.  The reason is that in the example given, the <final> state is not top-level (not a child of <scxml>), so that the state machine is still not supposed to set 'running' to false and halt.  However, I still think that there would be a problem if it were top level, for reasons outlined in the email below.  Please let me know if I'm wrong.
>> 
>> - Jim
>> 
>> -----Original Message-----
>> From: Jim Barnett 
>> Sent: Tuesday, January 29, 2013 10:42 AM
>> To: Jim Barnett; Voice Browser Working Group; www-voice@w3.org
>> Subject: RE: ISSUE-827: bug in detecting final states [SCXML-LC-Comments]
>> 
>> Here is one possible solution.  The current problem arises because when we end the 'while running and not stable' loop, running may be false, yet we continue on doing invokes, etc., and then waiting for an external event, with the result that we don't checking running until we have gotten an external event and returned to the top of the loop.  It looks to me like  the current algorithm will have  a problem whenever we enter a final state on an internal event with the external event queue empty.  
>> 
>> One fix would be to explicitly check for 'running' at the end of the 'while running and not stable' loop.  If we do that, we don't have to check running at the top of the loop.  Hence:
>> 
>> 
>> procedure mainEventLoop():
>> new=> while true:
>>       enabledTransitions = null
>>       stable = false
>>       # Here we handle eventless transitions and transitions 
>>       # triggered by internal events until machine is stable
>>       while running and not stable:
>>           enabledTransitions = selectEventlessTransitions()
>>           if enabledTransitions.isEmpty():
>>               if internalQueue.isEmpty(): 
>>                   stable = true
>>               else:
>>                   internalEvent = internalQueue.dequeue()
>>                   datamodel["_event"] = internalEvent
>>                   enabledTransitions = selectTransitions(internalEvent)
>>           if not enabledTransitions.isEmpty():
>>               microstep(enabledTransitions.toList())
>>       #check to see if we are in a top-level final state new=>  if not running:
>> new=>     break
>>       # Here we invoke whatever needs to be invoked. The implementation of 'invoke' is platform-specific
>>       for state in statesToInvoke:
>>           for inv in state.invoke:
>>               invoke(inv)
>>   etc...(rest of the loop not modified)
>> 
>> -----Original Message-----
>> From: Jim Barnett [mailto:Jim.Barnett@genesyslab.com]
>> Sent: Tuesday, January 29, 2013 10:01 AM
>> To: Voice Browser Working Group; www-voice@w3.org
>> Subject: RE: ISSUE-827: bug in detecting final states [SCXML-LC-Comments]
>> 
>> Here's a further detail to consider:  a state machine may enter a <final> child of <scxml> on any transition, even if there are still events in the internal event queue.  In that case, should the state machine process those events, or should it stop immediately?  In some sense, this is a distinction without a difference, because if the state machine is in a <final> child of <scxml>, no transitions are available to take.  An external observer will never see a difference between an implementation that processes the remaining events, and one that doesn't.  But at the algorithm/code level, the difference is significant.
>> 
>> 
>> 
>> -----Original Message-----
>> From: Voice Browser Working Group Issue Tracker [mailto:sysbot+tracker@w3.org]
>> Sent: Tuesday, January 29, 2013 9:11 AM
>> To: w3c-voice-wg@w3.org; www-voice@w3.org
>> Subject: ISSUE-827: bug in detecting final states [SCXML-LC-Comments]
>> 
>> ISSUE-827: bug in detecting final states [SCXML-LC-Comments]
>> 
>> https://www.w3.org/Voice/Group/track/issues/827
>> 
>> Raised by: James Barnett
>> On product: SCXML-LC-Comments
>> 
>>> From Gavin Kistner:
>> 
>> Consider this test case:
>> 
>>   <scxml xmlns="http://www.w3.org/2005/07/scxml" version="1.0" name="HistoryTest">
>>     <state id="s" initial="s1">
>>       <state id="s1">
>>         <onentry><raise event="go"/></onentry>
>>         <transition event="go" target="pass"/>
>>       </state>
>>       <final id="pass"/>
>>     </state>
>>   </scxml>
>> 
>> And this test runner:
>> 
>>   final2 = RXSCy.Machine(@data['final2']).start
>>   assert(final2.is_active?('pass'),"final2 should pass")
>>   refute(final2.running?,"final2 should not be running")
>> 
>> In my interpreter, the first assertion works, but the second fails. The state machine enters the 'pass' final state, but the `running` flag is never set to false.
>> 
>> Looking at the algorithm, I don't see where it this case is covered. There are only two spots where running is set to false. The first is when receiving an external cancel event during the mainEventLoop, and the other is at the end of enterStates:
>> 
>>   for s in configuration:
>>           if isFinalState(s) and isScxmlState(s.parent):
>>               running = false
>> 
>> 1) Is my test case wrong? Should entering the 'pass' final above not kill the interpreter?
>> 2) Or should the above pseudo-code be `if isInFinalState(s) and isScxmlState(s.parent)"`?
>> 
>> 
>> 
>> 
> 
> 

Received on Tuesday, 29 January 2013 21:05:05 UTC