[master] a80160a0c vcp: Check found pool just once in VCP_Ref()

Walid Boudebouda walid.boudebouda at gmail.com
Mon Apr 7 10:28:05 UTC 2025


commit a80160a0c5e5390ae9c81d790b157b5a7c05eb88
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Fri Mar 14 16:15:43 2025 +0100

    vcp: Check found pool just once in VCP_Ref()
    
    Testing cp2 nullity twice somehow resulted in multiple obj checks for
    both cp and cp2. In the cp case, there doesn't seem to be a point since
    the pool is created by the same function.
    
    Better diff with the --ignore-all-space option.

diff --git a/bin/varnishd/cache/cache_conn_pool.c b/bin/varnishd/cache/cache_conn_pool.c
index bca79783c..7e8519fbe 100644
--- a/bin/varnishd/cache/cache_conn_pool.c
+++ b/bin/varnishd/cache/cache_conn_pool.c
@@ -811,28 +811,23 @@ VCP_Ref(const struct vrt_endpoint *vep, const char *ident)
 	Lck_New(&cp->mtx, lck_conn_pool);
 	VTAILQ_INIT(&cp->connlist);
 
-	CHECK_OBJ_NOTNULL(cp, CONN_POOL_MAGIC);
 	Lck_Lock(&conn_pools_mtx);
 	cp2 = VRBT_FIND(vrb, &conn_pools, cp);
-	if (cp2 == NULL)
-		AZ(VRBT_INSERT(vrb, &conn_pools, cp));
-	else {
-		CHECK_OBJ(cp2, CONN_POOL_MAGIC);
-		assert(cp2->refcnt > 0);
-		cp2->refcnt++;
-	}
-	Lck_Unlock(&conn_pools_mtx);
-
 	if (cp2 == NULL) {
-		CHECK_OBJ_NOTNULL(cp, CONN_POOL_MAGIC);
+		AZ(VRBT_INSERT(vrb, &conn_pools, cp));
+		Lck_Unlock(&conn_pools_mtx);
 		return (cp);
 	}
 
+	CHECK_OBJ(cp2, CONN_POOL_MAGIC);
+	assert(cp2->refcnt > 0);
+	cp2->refcnt++;
+	Lck_Unlock(&conn_pools_mtx);
+
 	Lck_Delete(&cp->mtx);
 	AZ(cp->n_conn);
 	AZ(cp->n_kill);
 	FREE_OBJ(cp->endpoint);
 	FREE_OBJ(cp);
-	CHECK_OBJ_NOTNULL(cp2, CONN_POOL_MAGIC);
 	return (cp2);
 }


More information about the varnish-commit mailing list