I have a question about abort handling and hope that you remember
how
this was meant to work. Did we want to abort instead of sending a
message that was just generated by the Synthesis engine?
According to your code comment, this was the goal:
//check for abort, if so, modify step command for next step.
//We think abort is useful when the server is unresponsive or
//too slow to the user; therefore, we can delay abort at other
//points to this two points (before sending and before receiving
//the data).
if (checkForAbort() && (stepCmd ==
sysync::STEPCMD_RESENDDATA
|| stepCmd ==sysync::STEPCMD_SENDDATA
|| stepCmd == sysync::STEPCMD_NEEDDATA)) {
stepCmd = sysync::STEPCMD_ABORT;
}
I tried to abort before sending a message via some other means (script
sets a flag, flag gets checked in addition to checkForAbort()) and found
that this didn't work:
* SessionStep() returns STEPCMD_SENDDATA
* our case statement for that starts sending, then sets
STEPCMD_SENTDATA
* the code above is invoked, but doesn't abort
* SessionStep() is called with STEPCMD_SENTDATA, returns
STEPCMD_NEEDDATA
* our case statement for that waits, eventually might end up in
TransportAgent::FAILED (which checks for abort by itself) or
TransportAgent::GOT_REPLY (which doesn't check for abort and
instead goes to STEPCMD_GOTDATA)
In none of these cases does the checkForAbort() quoted above have any
effect, as far as I can tell. Do you know in which case it is needed?
Do we miss a check for abort directly after SessionStep and/or should
the code above perhaps be moved there?
I think we might miss a check. I remember once I struggled between testing
against
STEPCMD_SENDDATA or STEPCMD_SENTDATA and changed to SENDDATA at last.
Reviewing the code however doesn't give me the confidence any more. Probably testing
Against SENDDATA is more appropriate.
To be safe, I suggest let's test both.
See following commit:
1da586f3850dcf4e72dfba0e82cb75e3558cae20
Aborting: allow aborting directly before sending out the request
Abort before sending: the case statement will not change the state and start aborting in
the next iteration
Abort just after sending: the state will be SENTDATA and got caught
Abort while waiting: the transport is expected to check the abort flags itself, it is also
possible that the transport stops wait without changing it's state (NEEDDATA), will be
caught in next iteration.
Abort before resending: caught by RESENDDATA.
--
Best Regards,
Congwu