Re: [jgroups-dev] Race condition with RspFilter

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

-- 
Bela Ban
Lead JGroups / JBoss Clustering team
JBoss - a division of 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