[4.0] 9c490b6 Add a final backstop, so we absolutely 100% certainly do not try to delete a objhead while it still has a waiting list, by forcing the last ref holder to rush the WL.

Martin Blix Grydeland martin at varnish-software.com
Mon Jan 25 15:59:37 CET 2016


commit 9c490b6cfa3a5c3912987c8fdc6fea1aaf845ab7
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Thu Jan 7 00:03:35 2016 +0000

    Add a final backstop, so we absolutely 100% certainly do not
    try to delete a objhead while it still has a waiting list,
    by forcing the last ref holder to rush the WL.
    
    Since the hasher owns the refcounts on objhead, we cannot just
    mingle req and objcore refcounts.
    
    Fortunately we don't need to add another refcounter, because all
    we really care about is the wl being empty when we drop the last
    ref.
    
    The wl/hsh_rush() mechanism will work differently with different
    thread-scheduling schenarios, and I cannot definitively rule out
    that we can drop the last ref on an oh, while there are still req's
    on the waiting list.
    
    Given that, and the existence proof in ticket #1823's race, this
    might have been the indicated memory-trampler.
    
    Conflicts:
    	bin/varnishd/cache/cache_hash.c

diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index a948372..7cf085d 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -864,11 +864,26 @@ HSH_DerefObjHead(struct dstat *ds, struct objhead **poh)
 		oh->refcnt--;
 		Lck_Unlock(&oh->mtx);
 		return(1);
-	} else if (oh->waitinglist != NULL) {
+	}
+
+	/*
+	 * Make absolutely certain that we do not let the final ref
+	 * disappear until the waitinglist is empty.
+	 * This is necessary because the req's on the waiting list do
+	 * not hold any ref on the objhead of their own, and we cannot
+	 * just make the hold the same ref's as objcore, that would
+	 * confuse hashers.
+	 */
+	while (oh->waitinglist != NULL) {
 		Lck_Lock(&oh->mtx);
+		assert(oh->refcnt > 0);
+		r = oh->refcnt;
 		if (oh->waitinglist != NULL)
 			hsh_rush(ds, oh);
 		Lck_Unlock(&oh->mtx);
+		if (r > 1)
+			break;
+		usleep(100000);
 	}
 
 	assert(oh->refcnt > 0);
diff --git a/bin/varnishd/hash/hash_critbit.c b/bin/varnishd/hash/hash_critbit.c
index ff148ba..1496797 100644
--- a/bin/varnishd/hash/hash_critbit.c
+++ b/bin/varnishd/hash/hash_critbit.c
@@ -394,9 +394,7 @@ hcb_start(void)
 static int __match_proto__(hash_deref_f)
 hcb_deref(struct objhead *oh)
 {
-	int r;
 
-	r = 1;
 	CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
 	Lck_Lock(&oh->mtx);
 	assert(oh->refcnt > 0);
@@ -413,7 +411,7 @@ hcb_deref(struct objhead *oh)
 #ifdef PHK
 	fprintf(stderr, "hcb_defef %d %d <%s>\n", __LINE__, r, oh->hash);
 #endif
-	return (r);
+	return (1);
 }
 
 static struct objhead * __match_proto__(hash_lookup_f)



More information about the varnish-commit mailing list