From 55dbeea07680f11f0b39bd21bb64a87e5e506b9e Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Mon, 27 Aug 2012 14:33:47 -0400 Subject: [PATCH] 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 Reviewed-by: Derrick Brashear --- src/rx/rx.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/rx/rx.c b/src/rx/rx.c index de97a3140..d3872c997 100644 --- 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 */ -- 2.39.5