From 3ad9c5583845986979a25d615a9640770a3bf9f6 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Fri, 16 Apr 2010 22:42:34 -0400 Subject: [PATCH] Rx: make conn_call_lock and conn_data_lock usage consistent The rx_connection.flags field is protected by the conn_data_lock but the conn_data_lock is not held everywhere the conn flags field is altered. This produces a race that can result in a deadlock when waiter flags are inadvertently prevented from being cleared. The conn_call_lock usage in rx_EndCall which was removed in Change e169708681eb1bbbb31951b95f68e861a4b01c7e must be restored. If rx_EndCall never obtains the conn_call_lock it can't ensure that the thread in rx_NewCall actively checking the calls will not end up blocking when there is now a call channel that can be reused. This usage of conn_call_lock can be removed only if a true producer/consumer model is implemented. Change-Id: Id093fd6c4a42afb833c775411be0cd406abf4ef7 Reviewed-on: http://gerrit.openafs.org/1766 Reviewed-by: Jeffrey Altman Tested-by: Jeffrey Altman Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- src/rx/rx.c | 11 ++++++++++- src/rx/rx.h | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/rx/rx.c b/src/rx/rx.c index 9ff827f..14d685c 100644 --- a/src/rx/rx.c +++ b/src/rx/rx.c @@ -2076,6 +2076,9 @@ rx_EndCall(struct rx_call *call, afs_int32 rc) * have checked this call, found it active and by the time it * goes to sleep, will have missed the signal. */ + MUTEX_EXIT(&call->lock); + MUTEX_ENTER(&conn->conn_call_lock); + MUTEX_ENTER(&call->lock); MUTEX_ENTER(&conn->conn_data_lock); conn->flags |= RX_CONN_BUSY; if (conn->flags & RX_CONN_MAKECALL_WAITING) { @@ -2116,7 +2119,10 @@ rx_EndCall(struct rx_call *call, afs_int32 rc) CALL_RELE(call, RX_CALL_REFCOUNT_BEGIN); MUTEX_EXIT(&call->lock); if (conn->type == RX_CLIENT_CONNECTION) { + MUTEX_ENTER(&conn->conn_data_lock); conn->flags &= ~RX_CONN_BUSY; + MUTEX_EXIT(&conn->conn_data_lock); + MUTEX_EXIT(&conn->conn_call_lock); } USERPRI; /* @@ -2385,8 +2391,8 @@ rxi_FreeCall(struct rx_call *call) * If someone else destroys a connection, they either have no * call lock held or are going through this section of code. */ + MUTEX_ENTER(&conn->conn_data_lock); 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); #ifdef RX_ENABLE_LOCKS @@ -2397,6 +2403,8 @@ rxi_FreeCall(struct rx_call *call) #else /* RX_ENABLE_LOCKS */ rxi_DestroyConnection(conn); #endif /* RX_ENABLE_LOCKS */ + } else { + MUTEX_EXIT(&conn->conn_data_lock); } } @@ -3153,6 +3161,7 @@ rxi_IsConnInteresting(struct rx_connection *aconn) if (aconn->flags & (RX_CONN_MAKECALL_WAITING | RX_CONN_DESTROY_ME)) return 1; + for (i = 0; i < RX_MAXCALLS; i++) { tcall = aconn->call[i]; if (tcall) { diff --git a/src/rx/rx.h b/src/rx/rx.h index 343a18e..b51822a 100644 --- a/src/rx/rx.h +++ b/src/rx/rx.h @@ -254,7 +254,7 @@ struct rx_connection { struct rx_service *service; /* used by servers only */ u_short serviceId; /* To stamp on requests (clients only) */ afs_uint32 refCount; /* Reference count */ - u_char flags; /* Defined below */ + u_char flags; /* Defined below - (conn_data_lock) */ u_char type; /* Type of connection, defined below */ u_char secondsUntilPing; /* how often to ping for each active call */ u_char securityIndex; /* corresponds to the security class of the */ -- 1.9.4