[master] c1bc0d8 Snapshot & refcount the final ban to check, to avoid running of the ban-list if the lurker washes this OC while we check it.

Poul-Henning Kamp phk at FreeBSD.org
Tue Mar 1 22:51:19 CET 2016


commit c1bc0d8ecee281d585d23173921ee8676421462f
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Tue Mar 1 21:50:15 2016 +0000

    Snapshot & refcount the final ban to check, to avoid running of the
    ban-list if the lurker washes this OC while we check it.
    
    Mostly diagnosed by:	Martin
    
    Fixes:	#1864

diff --git a/bin/varnishd/cache/cache_ban.c b/bin/varnishd/cache/cache_ban.c
index 949ee93..490dbf8 100644
--- a/bin/varnishd/cache/cache_ban.c
+++ b/bin/varnishd/cache/cache_ban.c
@@ -524,7 +524,7 @@ BAN_CheckObject(struct worker *wrk, struct objcore *oc, struct req *req)
 {
 	struct ban *b;
 	struct vsl_log *vsl;
-	struct ban * volatile b0;
+	struct ban *b0, *bn;
 	unsigned tests;
 
 	CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
@@ -545,18 +545,23 @@ BAN_CheckObject(struct worker *wrk, struct objcore *oc, struct req *req)
 	/* If that fails, make a safe check */
 	Lck_Lock(&ban_mtx);
 	b0 = ban_start;
+	bn = oc->ban;
+	bn->refcount++;
 	Lck_Unlock(&ban_mtx);
 
-	if (b0 == oc->ban)
+	AN(bn);
+
+	if (b0 == bn)
 		return (0);
 
+
 	/*
 	 * This loop is safe without locks, because we know we hold
 	 * a refcount on a ban somewhere in the list and we do not
 	 * inspect the list past that ban.
 	 */
 	tests = 0;
-	for (b = b0; b != oc->ban; b = VTAILQ_NEXT(b, list)) {
+	for (b = b0; b != bn; b = VTAILQ_NEXT(b, list)) {
 		CHECK_OBJ_NOTNULL(b, BAN_MAGIC);
 		if (b->flags & BANS_FLAG_COMPLETED)
 			continue;
@@ -565,6 +570,7 @@ BAN_CheckObject(struct worker *wrk, struct objcore *oc, struct req *req)
 	}
 
 	Lck_Lock(&ban_mtx);
+	bn->refcount--;
 	VSC_C_main->bans_tested++;
 	VSC_C_main->bans_tests_tested += tests;
 
diff --git a/bin/varnishd/cache/cache_ban_lurker.c b/bin/varnishd/cache/cache_ban_lurker.c
index 3e355c1..bca5dde 100644
--- a/bin/varnishd/cache/cache_ban_lurker.c
+++ b/bin/varnishd/cache/cache_ban_lurker.c
@@ -60,6 +60,7 @@ ban_cleantail(void)
 		Lck_Lock(&ban_mtx);
 		b = VTAILQ_LAST(&ban_head, banhead_s);
 		if (b != VTAILQ_FIRST(&ban_head) && b->refcount == 0) {
+			assert(VTAILQ_EMPTY(&b->objcore));
 			if (b->flags & BANS_FLAG_COMPLETED)
 				VSC_C_main->bans_completed--;
 			if (b->flags & BANS_FLAG_OBJ)
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 1d9d798..21088f7 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -408,10 +408,8 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
 		if (oc->ttl <= 0.)
 			continue;
 
-		if (BAN_CheckObject(wrk, oc, req)) {
-			oc->flags |= OC_F_DYING;
+		if (BAN_CheckObject(wrk, oc, req))
 			continue;
-		}
 
 		if (ObjHasAttr(wrk, oc, OA_VARY)) {
 			vary = ObjGetAttr(wrk, oc, OA_VARY, NULL);



More information about the varnish-commit mailing list