rx: dec rx_nWaiting on clearing RX_CALL_WAIT_PROC

Currently, a couple of callers (rxi_ResetCall, and
rxi_AttachServerProc) will decrement rx_nWaiting only if
RX_CALL_WAIT_PROC is set for a call, and the call is on a queue
(presumably rx_incomingCallQueue). This can cause an imbalance in
rx_nWaiting if these code paths are reached when, in another thread,
rx_GetCall has removed the call from its queue, but it has not yet
cleared RX_CALL_WAIT_PROC (this can happen while it is waiting for
call->lock). In this situation, rx_GetCall will remove the call from
its queue, wait, and e.g. rxi_ResetCall will clear RX_CALL_WAIT_PROC;
neither will decrement rx_nWaiting.

This is possible if a new call is started on a call channel with an
extant call that is waiting for a thread; we will rxi_ResetCall in
rxi_ReceivePacket, but rx_GetCall may be running at the same time.
This race may also be possible via rxi_AttachServerProc via
rxi_UpdatePeerReach -> TryAttach -> rxi_AttachServerProc while
rx_GetCall is running, but I'm not sure.

To avoid this, decrement rx_nWaiting based on RX_CALL_WAIT_PROC alone,
regardless of whether or not the call is on a queue. This mirrors the
incrementing rx_nWaiting behavior, where rx_nWaiting is only
incremented if RX_CALL_WAIT_PROC is unset for a call, so this should
guarantee that rx_nWaiting does not become unbalanced.

Backport of commit 660720d1f54a867e21f78b6ec4c024235e4c37b7

Change-Id: I3372e053d284e10702971769487a7580a6842ef2
Reviewed-on: http://gerrit.openafs.org/8015
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>

diff --git a/src/rx/rx.c b/src/rx/rx.c
index de97a31409dd8512295b2a8128153da9c4ab9595..d3872c997fa670ccbb2842bde146fcfd28aeb012 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -4922,12 +4922,11 @@ rxi_AttachServerProc(struct rx_call *call,
        if (call->flags & RX_CALL_WAIT_PROC) {
            /* Conservative:  I don't think this should happen */
            call->flags &= ~RX_CALL_WAIT_PROC;
+           MUTEX_ENTER(&rx_waiting_mutex);
+           rx_nWaiting--;
+           MUTEX_EXIT(&rx_waiting_mutex);
            if (queue_IsOnQueue(call)) {
                queue_Remove(call);
-
-                MUTEX_ENTER(&rx_waiting_mutex);
-                rx_nWaiting--;
-                MUTEX_EXIT(&rx_waiting_mutex);
            }
        }
        call->state = RX_STATE_ACTIVE;
@@ -5429,6 +5428,11 @@ rxi_ResetCall(struct rx_call *call, int newcall)
        osi_rxWakeup(&call->twind);
 #endif
 
+    if (flags & RX_CALL_WAIT_PROC) {
+       MUTEX_ENTER(&rx_stats_mutex);
+       rx_nWaiting--;
+       MUTEX_EXIT(&rx_stats_mutex);
+    }
 #ifdef RX_ENABLE_LOCKS
     /* The following ensures that we don't mess with any queue while some
      * other thread might also be doing so. The call_queue_lock field is
@@ -5443,12 +5447,6 @@ rxi_ResetCall(struct rx_call *call, int newcall)
        MUTEX_ENTER(call->call_queue_lock);
        if (queue_IsOnQueue(call)) {
            queue_Remove(call);
-           if (flags & RX_CALL_WAIT_PROC) {
-
-                MUTEX_ENTER(&rx_waiting_mutex);
-                rx_nWaiting--;
-                MUTEX_EXIT(&rx_waiting_mutex);
-           }
        }
        MUTEX_EXIT(call->call_queue_lock);
        CLEAR_CALL_QUEUE_LOCK(call);
@@ -5456,8 +5454,6 @@ rxi_ResetCall(struct rx_call *call, int newcall)
 #else /* RX_ENABLE_LOCKS */
     if (queue_IsOnQueue(call)) {
        queue_Remove(call);
-       if (flags & RX_CALL_WAIT_PROC)
-           rx_nWaiting--;
     }
 #endif /* RX_ENABLE_LOCKS */