RE: state notation code flags

["Mark Rivers" <rivers@cars.uchicago.edu>]

Andrew,

Either I don't understand your analysis, or I don't agree with it.

Your suggestion is:

int abort;
int abortOut;
assign abort to "{P}{R}Abort.VAL";
assign abortOut to "{P}{R}Abort.VAL";
monitor abort;
evflag abortMon;
sync abort abortMon;
...
ss xpsTrajectoryAbort {
    state monitorAbort {
        when (efTestAndClear(abortMon) &&
              (execState==EXECUTE_STATE_EXECUTING)) {
...
            abortOut=0;
            pvPut(abortOut);
        } state monitorAbort
    }
}

Consider what happens when an external agent sets Abort.VAL to 1.
The when statement will detect the monitor event, set abortOut=0 and
call pvPut.  That will result in another monitor event, and the when
statement will execute a second time, again setting abortOut=0 and doing
the pvPut.  In fact this can potentially cause an infinite loop, but
normally won't because records won't call dbPostEvents if the value has
not changed?

This double-processing is what Patrick is trying to avoid.  It can be
avoided by detecting that the monitor callback is for the same value
that was written with the previous pvPut, or by using Tim's database
logic.  

I'm probably missing something?

Mark


-----Original Message-----
From: Andrew Johnson [mailto:anj@aps.anl.gov] 
Sent: Monday, October 05, 2009 10:16 AM
To: tech-talk@aps.anl.gov
Cc: Mark Rivers; Patrick Thomas
Subject: Re: state notation code flags

Gentlemen,

On Saturday 03 October 2009 08:05:37 Mark Rivers wrote:
>
> What is it you are trying to do?  You are doing both pvPuts and
looking for
> external changes to the same PV?
>
> If so, then one solution is to ignore events which correspond to the
value
> that your SNL program set with pvPut, and only act on events with
values
> that must have been done by some external agent.  Here is an example
from
> one of my programs:
> int abort;
> assign abort to "{P}{R}Abort.VAL";
> monitor abort;
> evflag abortMon;
> sync abort abortMon;
> ...
> ss xpsTrajectoryAbort {
>     state monitorAbort {
>         when ((efTestAndClear(abortMon)) && (abort==1) &&
>               (execState==EXECUTE_STATE_EXECUTING)) {
> ...
>             abort=0;
>             pvPut(abort);
>         } state monitorAbort
>     }
> }

I believe the issue you are both discussing is a classic case of a race 
condition which is caused by doing exactly what Mark discusses.  I
suspect 
it's possible that the monitor callback can occur and change the value
of the 
abort variable above in between the lines abort=0 and pvPut(abort), but
even 
if that's not possible it is still causing the problems that Patrick is 
seeing.  It is unfortunate that the design of the SNL language makes
this 
possible, and it may require some major surgery to fix this in the 
implementation.

However there is an easy solution, which I'm pretty sure will fix
Patrick's 
issue: Add a second variable pointing to the same PV which is not
monitored 
and is used only to do the pvPut operations.  Copying Mark's example,
I've 
added the abortOut variable and removed the test of the abort value from
the 
when() condition, although it's possible that you might still want that.

int abort;
int abortOut;
assign abort to "{P}{R}Abort.VAL";
assign abortOut to "{P}{R}Abort.VAL";
monitor abort;
evflag abortMon;
sync abort abortMon;
...
ss xpsTrajectoryAbort {
    state monitorAbort {
        when (efTestAndClear(abortMon) &&
              (execState==EXECUTE_STATE_EXECUTING)) {
...
            abortOut=0;
            pvPut(abortOut);
        } state monitorAbort
    }
}

If you adopt this model, you're never changing the value of the abort
variable 
yourself, and I think you'll find that the result will only set the
event flag 
once, when your put value makes its way back to you via the monitor.

I suspect it would be a good idea to try and get the the SNL compiler to
issue 
a warning whenever it sees someone calling pvPut() on a variable which
has a 
monitor on it, since this is IMHO an unsafe practice.

- Andrew
-- 
The best FOSS code is written to be read by other humans -- Harald Welte