[jgroups-dev] Race condition with RspFilter

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