Re: [jgroups-dev] Race condition with RspFilter

Hmmm, maybe you could have a RspFilter2 interface with this and deprecate RspFilter?

----- "Bela Ban" <[email protected]> wrote:

> Problem is this is an API change...
> 
> Vladimir Blagojevic wrote:
> > Brian and I had a brief chat on IIRC regarding this issue. How about
> having only one method in RspFilter:
> > RspFilerResult responseReceived(Object response, Address sender);
> >
> > RspFilterResult is an enum with four states where each state is
> essentially a combination of two boolean variables: validResponse and
> needMoreResponses.
> >
> > Cheers
> > On 2010-04-15, at 6:29 PM, Brian Stansberry wrote:
> >
> >   
> >> I think the RspFilter handling has a small race. (Writing this has
> made 
> >> me think it's very small, but since it's 1/2 written...)
> >>
> >> Consider this simple RspFilter implementation that signals that no
> 
> >> further responses are needed once a non-null response is received:
> >>
> >> public class NonNullFilter implements RspFilter
> >> {
> >>    private volatile boolean validResponse;
> >>
> >>    public boolean isAcceptable(Object response, Address sender)
> >>    {
> >>       if (response != null)
> >>       {
> >>          validResponse = true;
> >>       }
> >>
> >>       return true;
> >>    }
> >>
> >>    public boolean needMoreResponses()
> >>    {
> >>       return !(validResponse);
> >>    }
> >> }
> >>
> >> I think that captures the basic use case of RspFilter.
> >>
> >> The handling of response is like this in
> GroupRequest.responseReceived():
> >>
> >>         boolean responseReceived=false;
> >>         if(!rsp.wasReceived()) {
> >>             if((responseReceived=(rsp_filter == null) || 
> >> rsp_filter.isAcceptable(response_value, sender)))
> >>                 rsp.setValue(response_value);
> >>             rsp.setReceived(responseReceived);
> >>         }
> >>
> >>         lock.lock();
> >>         try {
> >>             if(responseReceived)
> >>                 num_received++;
> >>             done=rsp_filter == null? responsesComplete() : 
> >> !rsp_filter.needMoreResponses();
> >>             if(responseReceived || done)
> >>                 completed.signalAll(); // wakes up execute()
> >>
> >> Now imagine 2 responses arriving nearly concurrently, with thread
> T1 
> >> carrying a response value of null, and T2 carrying a non-null
> response.
> >>
> >> 1) T1 calls isAcceptable(), validResponse == false
> >> 2) T1 calls rsp.setValue()
> >> 3) T2 calls isAcceptable(), validResponse == true
> >> 4) T1 calls needMoreResponses, gets "false" since validResponse ==
> true
> >> 5) T1 calls completed.signalAll(), wakes up caller
> >> 6) Caller thread wakes up
> >> 7) Caller thread processes response list, doesn't see the T2 value
> as 
> >> it's not set yet
> >> 8) T2 calls rsp.setValue().
> >>
> >> Now that's pretty improbable, i.e. that steps 4,5,6,7 all execute 
> >> between 3 and 8. But it's a race.
> >>
> >> A semi-related thing is that NonNullFilter.isAcceptable() always
> returns 
> >> true. Seems counter-intuitive, why not return "false" if response
> == 
> >> null?  Reason is that num_received is only incremented if
> isAcceptable() 
> >> returns true. Effect is that if isAcceptable() doesn't always
> return 
> >> true, if only "null" responses are received 
> >> NonNullFilter.needMoreResponses() will never return false, *and* 
> >> GroupRequest.responsesComplete() will never return true!  The
> caller 
> >> thread will just wait until timeout
> >>
> >> I wonder if in 3.0 a simpler RspFilter API would just be a single
> method:
> >>
> >> boolean needMoreResponses((Object response, Address sender)
> >>
> >> That would make it easier to avoid the race. Is there a use case
> for not 
> >> marking a Rsp as received? That's what the separate 
> >> isResponseAcceptable() method allows.
> >>
> >> Apologies if this has been discussed before; I have a vague feeling
> it 
> >> has come up.
> >>
> >> -- 
> >> Brian Stansberry
> >> Lead, AS Clustering
> >> JBoss by Red Hat
> >>
> >>
> ------------------------------------------------------------------------------
> >> Download Intel&#174; Parallel Studio Eval
> >> Try the new software tools for yourself. Speed compiling, find
> bugs
> >> proactively, and fine-tune applications for parallel performance.
> >> See why Intel Parallel Studio got high marks during beta.
> >> http://p.sf.net/sfu/intel-sw-dev
> >> _______________________________________________
> >> Javagroups-development mailing list
> >>
> >>     
> >
> >
> >
> ------------------------------------------------------------------------------
> > Download Intel&#174; Parallel Studio Eval
> > Try the new software tools for yourself. Speed compiling, find bugs
> > proactively, and fine-tune applications for parallel performance.
> > See why Intel Parallel Studio got high marks during beta.
> > http://p.sf.net/sfu/intel-sw-dev
> > _______________________________________________
> > Javagroups-development mailing list
> >
> >
> >   
> 
> -- 
> Bela Ban
> Lead JGroups / JBoss Clustering team
> JBoss - a division of Red Hat
> 
> 
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> _______________________________________________
> Javagroups-development mailing list

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Javagroups-development mailing list