Re: [jgroups-dev] Race condition with RspFilter

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® 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® 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