r5304 - trunk/varnish-cache/bin/varnishd

[phk at varnish-cache.org]

Author: phk
Date: 2010-09-28 11:34:50 +0200 (Tue, 28 Sep 2010)
New Revision: 5304

Modified:
   trunk/varnish-cache/bin/varnishd/hash_critbit.c
Log:
After considerable thinking, I have come to the conclusion that this
assert is bogus.

There is a window from our lookup until we lock the refcount where
a different thread conceiveably could loose the last reference to
the objhdr, before we get the lock to increase it.

The situation the assert was meant to protect, where we insert 
a new objhdr, is handled in the "oh == noh" clause earlier, were
the weaker "> 0" check is used (rather than "== 1") to cater for
the exact same race.

Fixes: #761
Fixes: #783



Modified: trunk/varnish-cache/bin/varnishd/hash_critbit.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/hash_critbit.c	2010-09-28 09:28:24 UTC (rev 5303)
+++ trunk/varnish-cache/bin/varnishd/hash_critbit.c	2010-09-28 09:34:50 UTC (rev 5304)
@@ -468,12 +468,10 @@
 		 * under us, so fall through and try with the lock held.
 		 */
 		u = oh->refcnt;
-		if (u > 0) {
+		if (u > 0)
 			oh->refcnt++;
-		} else {
-			assert(!with_lock);
+		else 
 			with_lock = 1;
-		}
 		Lck_Unlock(&oh->mtx);
 		if (u > 0)
 			return (oh);