Rx: Avoid lastBusy/PEER_BUSY discrepancy
authorAndrew Deason <adeason@sinenomine.net>
Mon, 13 Feb 2012 20:11:36 +0000 (14:11 -0600)
committerDerrick Brashear <shadow@dementix.org>
Mon, 20 Feb 2012 15:54:45 +0000 (07:54 -0800)
If an rx call has the RX_CALL_PEER_BUSY flag set, but the call's
conn->lastBusy is not set, we can easily cause an rx caller to loop
infinitely. rx_NewCall will see that lastBusy for a call channel is
not set, and will use that call channel, but rxi_CheckBusy will note
that the call appears busy and that there are non-busy call channels
on the same conn, and so will return RX_CALL_BUSY.

This can currently happen in rxi_ResetCall, since we set
RX_CALL_PEER_BUSY on the call again if the call had that flag set when
rxi_ResetCall was called. If we are calling rxi_ResetCall with
'newcall' set, the passed in call is unrelated to the new call, since
it was obtained from the free list. Thus, the busy-ness of the call
should be ignored. Fix this by only paying attention to the incoming
RX_CALL_PEER_BUSY flag if 'newcall' is not set.

Also prevent this from happening by clearing RX_CALL_PEER_BUSY in
rx_NewCall when we select a call and clear lastBusy for that call.

Change-Id: Ic5a4709854b62d962ed91ee0103c6cbdd735d175
Reviewed-on: http://gerrit.openafs.org/6707
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Alistair Ferguson <alistair.ferguson@mac.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>

src/rx/rx.c

index 2a4580e..f62ffd1 100644 (file)
@@ -1551,6 +1551,7 @@ rx_NewCall(struct rx_connection *conn)
        }
        if (i < RX_MAXCALLS) {
            conn->lastBusy[i] = 0;
+           call->flags &= ~RX_CALL_PEER_BUSY;
            break;
        }
         if (!wait)
@@ -5255,9 +5256,13 @@ rxi_ResetCall(struct rx_call *call, int newcall)
     }
     call->flags = 0;
 
-    if ((flags & RX_CALL_PEER_BUSY)) {
+    if (!newcall && (flags & RX_CALL_PEER_BUSY)) {
        /* The call channel is still busy; resetting the call doesn't change
-        * that */
+        * that. However, if 'newcall' is set, we are processing a call
+        * structure that has either been recycled from the free list, or has
+        * been newly allocated. So, RX_CALL_PEER_BUSY is not relevant if
+        * 'newcall' is set, since it describes a completely different call
+        * channel which we do not care about. */
        call->flags |= RX_CALL_PEER_BUSY;
     }