jimzah/openafs
1
0
Fork 0
mirror of https://git.openafs.org/openafs.git synced 2025-01-18 15:00:12 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

rx: Reap client conns in rxi_ReapConnections

Browse Source 
rxi_DestroyConnectionNoLock contains the following early return
test:

    if ((conn->refCount > 0) || (conn->flags & RX_CONN_BUSY)) {
       /* Busy; wait till the last guy before proceeding */
       MUTEX_EXIT(&rx_refcnt_mutex);
       MUTEX_EXIT(&conn->conn_data_lock);
       USERPRI;
       return;
    }

This prevents the rx_connection ("conn") from being destroyed if it is
still in use elsewhere, e.g.:
 - the rx_Listener is handling packets for one of the conn's rx_calls
 - another caller has a reference to the conn

However, since the original IBM code import, rxi_ReceivePacket obtains a
conn reference (via rxi_FindConnection) but returns it by merely
decrementing the refCount (currently via putConnection), and does not
call rx_DestroyConnection.  If the listener's rxi_ReceivePacket is
processing a reply or ack for a client conn (holding a reference) while
another thread is calling rx_DestroyConnection on the otherwise-last
reference, the rx_connection will not be destroyed and can never be
destroyed - it has been leaked.

Modify rxi_ReapConnections to destroy client conns with refCount == 0.
This way, these connections can still "leak", but they're eventually
cleaned up the next time rxi_ReapConnections runs.

Change-Id: I8c588d43b108a8147a07d0ff0cc69055cd00d355
Reviewed-on: https://gerrit.openafs.org/15135
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
This commit is contained in:
Mark Vitale 2022-09-08 14:16:46 -04:00 committed by Andrew Deason
parent 4b5c803d8d
commit a5fc7b3fc3
1 changed files with 38 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
src/rx/rx.c
View File

@ -7179,8 +7179,12 @@ rxi_ComputeRoundTripTime(struct rx_packet *p,
}
/* Find all server connections that have not been active for a long time, and
* toss them */
/*
* Find and toss:
* - server connections that have not been active for a long time
* - client connections with refCount 0
*
*/
static void
rxi_ReapConnections(struct rxevent *unused, void *unused1, void *unused2,
int unused3)
@ -7204,6 +7208,8 @@ rxi_ReapConnections(struct rxevent *unused, void *unused1, void *unused2,
rereap:
for (conn = *conn_ptr; conn; conn = next) {
int destroyConn = 0;
/* XXX -- Shouldn't the connection be locked? */
next = conn->next;
havecalls = 0;
@ -7228,28 +7234,45 @@ rxi_ReapConnections(struct rxevent *unused, void *unused1, void *unused2,
}
if (havecalls)
continue;
if (conn->type == RX_SERVER_CONNECTION) {
/* This only actually destroys the connection if
* there are no outstanding calls */
/*
* Refcount 0 server conns are not currently in use, but
* could be reused if a new call arrives from the peer.
* Therefore we only destroy them here if they haven't been
* used in rx_idleConnectionTime seconds.
*/
MUTEX_ENTER(&conn->conn_data_lock);
MUTEX_ENTER(&rx_refcnt_mutex);
if (!conn->refCount
&& ((conn->lastSendTime + rx_idleConnectionTime) <
now.sec)) {
conn->refCount++; /* it will be decr in rx_DestroyConn */
MUTEX_EXIT(&rx_refcnt_mutex);
MUTEX_EXIT(&conn->conn_data_lock);
destroyConn = 1;
}
MUTEX_EXIT(&rx_refcnt_mutex);
MUTEX_EXIT(&conn->conn_data_lock);
} else {
/*
* Refcount 0 client conns are no longer in use, and thus
* may be safely destroyed on sight. Normally a client
* conn would be destroyed when the last reference is
* dropped, but not all callers are diligent to do that, so
* this is where we clean up any stragglers.
*/
MUTEX_ENTER(&rx_refcnt_mutex);
if (conn->refCount == 0) {
conn->refCount++; /* will be decr in rx_DestroyConn */
destroyConn = 1;
}
MUTEX_EXIT(&rx_refcnt_mutex);
}
if (destroyConn) {
#ifdef RX_ENABLE_LOCKS
rxi_DestroyConnectionNoLock(conn);
rxi_DestroyConnectionNoLock(conn);
#else /* RX_ENABLE_LOCKS */
rxi_DestroyConnection(conn);
#endif /* RX_ENABLE_LOCKS */
}
#ifdef RX_ENABLE_LOCKS
else {
MUTEX_EXIT(&rx_refcnt_mutex);
MUTEX_EXIT(&conn->conn_data_lock);
}
rxi_DestroyConnection(conn);
#endif /* RX_ENABLE_LOCKS */
}
}