STABLE12-rx-makecall-race-fix-20050518

On at least one system it was noticed that threads waiting in rx_NewCall
would starve forever (aka deadlock).   This was the result of one out of
two problems related to a race condition on the RX_CONN_MAKECALL_WAITING
bit flag.  This flag was set once in rx_NewCall and cleared in rx_EndCall.
However, it was possible for the flag to be cleared even though there
were additional flags waiting in rx_NewCall.  This was due to a failure
to check the value of makeCallWaiters before clearing the flag and also
due to a failure to properly lock the access to the makeCallWaiters field.

The second problem was an ability to destroy a connection on which threads
are waiting within rx_NewCall.


(cherry picked from commit 10f6e5d6e2)
This commit is contained in:
Jeffrey Altman 2005-05-18 23:13:32 +00:00
parent 852b1486f3
commit 1b27bc1f54

View File

@ -1037,14 +1037,32 @@ struct rx_call *rx_NewCall(conn)
* If so, let them go first to avoid starving them.
* This is a fairly simple scheme, and might not be
* a complete solution for large numbers of waiters.
*
* makeCallWaiters keeps track of the number of
* threads waiting to make calls and the
* RX_CONN_MAKECALL_WAITING flag bit is used to
* indicate that there are indeed calls waiting.
* The flag is set when the waiter is incremented.
* It is only cleared in rx_EndCall when
* makeCallWaiters is 0. This prevents us from
* accidently destroying the connection while it
* is potentially about to be used.
*/
MUTEX_ENTER(&conn->conn_data_lock);
if (conn->makeCallWaiters) {
conn->flags |= RX_CONN_MAKECALL_WAITING;
conn->makeCallWaiters++;
MUTEX_EXIT(&conn->conn_data_lock);
#ifdef RX_ENABLE_LOCKS
CV_WAIT(&conn->conn_call_cv, &conn->conn_call_lock);
CV_WAIT(&conn->conn_call_cv, &conn->conn_call_lock);
#else
osi_rxSleep(conn);
osi_rxSleep(conn);
#endif
}
MUTEX_ENTER(&conn->conn_data_lock);
conn->makeCallWaiters--;
}
MUTEX_EXIT(&conn->conn_data_lock);
for (;;) {
for (i=0; i<RX_MAXCALLS; i++) {
@ -1068,15 +1086,17 @@ struct rx_call *rx_NewCall(conn)
}
MUTEX_ENTER(&conn->conn_data_lock);
conn->flags |= RX_CONN_MAKECALL_WAITING;
conn->makeCallWaiters++;
MUTEX_EXIT(&conn->conn_data_lock);
conn->makeCallWaiters++;
#ifdef RX_ENABLE_LOCKS
CV_WAIT(&conn->conn_call_cv, &conn->conn_call_lock);
#else
osi_rxSleep(conn);
#endif
MUTEX_ENTER(&conn->conn_data_lock);
conn->makeCallWaiters--;
MUTEX_EXIT(&conn->conn_data_lock);
}
/*
* Wake up anyone else who might be giving us a chance to
@ -1828,6 +1848,9 @@ afs_int32 rx_EndCall(call, rc)
* rx_NewCall is in a stable state. Otherwise, rx_NewCall may
* have checked this call, found it active and by the time it
* goes to sleep, will have missed the signal.
*
* Do not clear the RX_CONN_MAKECALL_WAITING flag as long as
* there are threads waiting to use the conn object.
*/
MUTEX_EXIT(&call->lock);
MUTEX_ENTER(&conn->conn_call_lock);
@ -1835,7 +1858,8 @@ afs_int32 rx_EndCall(call, rc)
MUTEX_ENTER(&conn->conn_data_lock);
conn->flags |= RX_CONN_BUSY;
if (conn->flags & RX_CONN_MAKECALL_WAITING) {
conn->flags &= (~RX_CONN_MAKECALL_WAITING);
if (conn->makeCallWaiters == 0)
conn->flags &= (~RX_CONN_MAKECALL_WAITING);
MUTEX_EXIT(&conn->conn_data_lock);
#ifdef RX_ENABLE_LOCKS
CV_BROADCAST(&conn->conn_call_cv);
@ -2122,7 +2146,7 @@ void rxi_FreeCall(call)
* If someone else destroys a connection, they either have no
* call lock held or are going through this section of code.
*/
if (conn->flags & RX_CONN_DESTROY_ME) {
if (conn->flags & RX_CONN_DESTROY_ME && !(conn->flags & RX_CONN_MAKECALL_WAITING)) {
MUTEX_ENTER(&conn->conn_data_lock);
conn->refCount++;
MUTEX_EXIT(&conn->conn_data_lock);