From: Andrew Deason Date: Mon, 15 Feb 2010 16:55:33 +0000 (-0600) Subject: h_TossStuff_r: check held-ness after lock X-Git-Tag: openafs-devel-1_5_73~152 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=45aceee9842265fb0ccdb5e8f3f6d32c8d2b99ea;p=packages%2Fo%2Fopenafs.git h_TossStuff_r: check held-ness after lock h_TossStuff_r checks if a host is held or locked by another thread before trying to delete the host. Unfortunately, it checks if it is locked before checking if it is held, and the lock check drops H_LOCK. Thus, another thread could hold the host while we don't have H_LOCK, and we could delete a host that is being held. Although it is a bug if any thread holds a host that is being deleted, some instances of this still exist, so make the check more robust. Reverse the order of the tests, so we detect if someone held the host while the lock check dropped H_LOCK. Also log when this happens, as it indicates a bug occurring. FIXES 126454 Change-Id: I8fa972c430e63fc46ca4fadcc3429173ac91a947 Reviewed-on: http://gerrit.openafs.org/1312 Tested-by: Andrew Deason Reviewed-by: Alistair Ferguson Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- diff --git a/src/viced/host.c b/src/viced/host.c index 8796bddee..46d4ba93a 100644 --- a/src/viced/host.c +++ b/src/viced/host.c @@ -843,10 +843,6 @@ h_TossStuff_r(register struct host *host) { register struct client **cp, *client; - /* if somebody still has this host held */ - if (host->refCount > 0) - return; - /* if somebody still has this host locked */ if (h_NBLock_r(host) != 0) { char hoststr[16]; @@ -858,6 +854,17 @@ h_TossStuff_r(register struct host *host) h_Unlock_r(host); } + /* if somebody still has this host held */ + /* we must check this _after_ h_NBLock_r, since h_NBLock_r can drop and + * reacquire H_LOCK */ + if (host->refCount > 0) { + char hoststr[16]; + ViceLog(0, + ("Warning: h_TossStuff_r failed; Host %" AFS_PTR_FMT " (%s:%d) was held.\n", + host, afs_inet_ntoa_r(host->host, hoststr), ntohs(host->port))); + return; + } + /* ASSUMPTION: rxi_FreeConnection() does not yield */ for (cp = &host->FirstClient; (client = *cp);) { if ((host->hostFlags & HOSTDELETED) || client->deleted) {