[experimental-ims] f289def Make all objects have an objcore, even for the vcl_recv->pass case.

Poul-Henning Kamp phk at FreeBSD.org
Thu Dec 18 10:27:47 CET 2014


commit f289def3a62bdc15aa0ef78217254edba689ef2f
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon May 14 10:05:53 2012 +0000

    Make all objects have an objcore, even for the vcl_recv->pass case.
    
    This simplifies some nasty corner cases.

diff --git a/bin/varnishd/cache/cache_ban.c b/bin/varnishd/cache/cache_ban.c
index c53d945..6986d21 100644
--- a/bin/varnishd/cache/cache_ban.c
+++ b/bin/varnishd/cache/cache_ban.c
@@ -447,6 +447,7 @@ BAN_NewObjCore(struct objcore *oc)
 
 	CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
 	AZ(oc->ban);
+	AN(oc->objhead);
 	Lck_Lock(&ban_mtx);
 	oc->ban = ban_start;
 	ban_start->refcount++;
diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c
index 7fa0291..48efe08 100644
--- a/bin/varnishd/cache/cache_busyobj.c
+++ b/bin/varnishd/cache/cache_busyobj.c
@@ -152,7 +152,7 @@ VBO_DerefBusyObj(struct worker *wrk, struct busyobj **pbo)
 	*pbo = NULL;
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
 	CHECK_OBJ_ORNULL(bo->fetch_obj, OBJECT_MAGIC);
-	if (bo->fetch_obj != NULL && bo->fetch_obj->objcore != NULL) {
+	if (bo->fetch_obj != NULL && bo->fetch_obj->objcore->objhead != NULL) {
 		oc = bo->fetch_obj->objcore;
 		CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
 		CHECK_OBJ_NOTNULL(oc->objhead, OBJHEAD_MAGIC);
diff --git a/bin/varnishd/cache/cache_center.c b/bin/varnishd/cache/cache_center.c
index c028cb5..9b7a968 100644
--- a/bin/varnishd/cache/cache_center.c
+++ b/bin/varnishd/cache/cache_center.c
@@ -256,7 +256,7 @@ cnt_prepresp(struct sess *sp, struct worker *wrk, struct req *req)
 	}
 
 	req->t_resp = W_TIM_real(wrk);
-	if (req->obj->objcore != NULL) {
+	if (req->obj->objcore->objhead != NULL) {
 		if ((req->t_resp - req->obj->last_lru) >
 		    cache_param->lru_timeout &&
 		    EXP_Touch(req->obj->objcore))
@@ -340,11 +340,8 @@ cnt_deliver(struct sess *sp, struct worker *wrk, struct req *req)
 	RES_WriteObj(sp);
 
 	/* No point in saving the body if it is hit-for-pass */
-	if (req->obj->objcore != NULL) {
-		CHECK_OBJ_NOTNULL(req->obj->objcore, OBJCORE_MAGIC);
-		if (req->obj->objcore->flags & OC_F_PASS)
-			STV_Freestore(req->obj);
-	}
+	if (req->obj->objcore->flags & OC_F_PASS)
+		STV_Freestore(req->obj);
 
 	assert(WRW_IsReleased(wrk));
 	(void)HSH_Deref(&wrk->stats, NULL, &req->obj);
@@ -492,6 +489,7 @@ cnt_error(struct sess *sp, struct worker *wrk, struct req *req)
 	bo->vsl->wid = sp->vsl_id;
 	AZ(bo->stats);
 	bo->stats = &wrk->stats;
+	req->objcore = HSH_NewObjCore(wrk);
 	req->obj = STV_NewObject(bo, &req->objcore,
 	    TRANSIENT_STORAGE, cache_param->http_resp_size,
 	    (uint16_t)cache_param->http_max_hdr);
@@ -586,7 +584,7 @@ cnt_fetch(struct sess *sp, struct worker *wrk, struct req *req)
 
 	wrk->acct_tmp.fetch++;
 
-	i = FetchHdr(sp, need_host_hdr, req->objcore == NULL);
+	i = FetchHdr(sp, need_host_hdr, req->objcore->objhead == NULL);
 	/*
 	 * If we recycle a backend connection, there is a finite chance
 	 * that the backend closed it before we get a request to it.
@@ -594,7 +592,7 @@ cnt_fetch(struct sess *sp, struct worker *wrk, struct req *req)
 	 */
 	if (i == 1) {
 		VSC_C_main->backend_retry++;
-		i = FetchHdr(sp, need_host_hdr, req->objcore == NULL);
+		i = FetchHdr(sp, need_host_hdr, req->objcore->objhead == NULL);
 	}
 
 	if (i) {
@@ -626,7 +624,7 @@ cnt_fetch(struct sess *sp, struct worker *wrk, struct req *req)
 		RFC2616_Ttl(bo, sp->req->xid);
 
 		/* pass from vclrecv{} has negative TTL */
-		if (req->objcore == NULL)
+		if (req->objcore->objhead == NULL)
 			bo->exp.ttl = -1.;
 
 		AZ(bo->do_esi);
@@ -634,7 +632,7 @@ cnt_fetch(struct sess *sp, struct worker *wrk, struct req *req)
 
 		VCL_fetch_method(sp);
 
-		if (req->objcore != NULL && bo->do_pass)
+		if (bo->do_pass)
 			req->objcore->flags |= OC_F_PASS;
 
 		switch (req->handling) {
@@ -652,7 +650,7 @@ cnt_fetch(struct sess *sp, struct worker *wrk, struct req *req)
 	/* Clean up partial fetch */
 	AZ(bo->vbc);
 
-	if (req->objcore != NULL) {
+	if (req->objcore->objhead != NULL || req->handling == VCL_RET_ERROR) {
 		CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
 		AZ(HSH_Deref(&wrk->stats, req->objcore, NULL));
 		req->objcore = NULL;
@@ -707,7 +705,7 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
 
 	assert(req->handling == VCL_RET_DELIVER);
 
-	if (req->objcore == NULL) {
+	if (req->objcore->objhead == NULL) {
 		/* This is a pass from vcl_recv */
 		pass = 1;
 		/* VCL may have fiddled this, but that doesn't help */
@@ -785,7 +783,7 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
 	    pass ? HTTPH_R_PASS : HTTPH_A_INS, &nhttp);
 
 	/* Create Vary instructions */
-	if (req->objcore != NULL) {
+	if (req->objcore->objhead != NULL) {
 		CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
 		vary = VRY_Create(req, bo->beresp);
 		if (vary != NULL) {
@@ -884,7 +882,7 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
 
 	assert(bo->refcount == 2);	/* one for each thread */
 
-	if (req->obj->objcore != NULL) {
+	if (req->obj->objcore->objhead != NULL) {
 		EXP_Insert(req->obj);
 		AN(req->obj->objcore->ban);
 		AZ(req->obj->ws_o->overflow);
@@ -895,7 +893,7 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
 	    Pool_Task(wrk->pool, &bo->fetch_task, POOL_NO_QUEUE))
 		FetchBody(wrk, bo);
 
-	if (req->obj->objcore != NULL)
+	if (req->obj->objcore->objhead != NULL)
 		HSH_Ref(req->obj->objcore);
 
 	if (bo->state == BOS_FINISHED) {
@@ -1249,6 +1247,9 @@ cnt_pass(struct sess *sp, struct worker *wrk, struct req *req)
 	assert(req->handling == VCL_RET_PASS);
 	wrk->acct_tmp.pass++;
 	sp->step = STP_FETCH;
+
+	req->objcore = HSH_NewObjCore(wrk);
+	req->objcore->busyobj = bo;
 	return (0);
 }
 
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index ee9147d..7e9a25c 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -453,10 +453,8 @@ FetchHdr(struct sess *sp, int need_host_hdr, int sendbody)
 	AN(req->director);
 	AZ(req->obj);
 
-	if (req->objcore != NULL) {		/* pass has no objcore */
-		CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
-		AN(req->objcore->flags & OC_F_BUSY);
-	}
+	CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
+	AN(req->objcore->flags & OC_F_BUSY);
 
 	hp = bo->bereq;
 
@@ -687,7 +685,7 @@ FetchBody(struct worker *wrk, void *priv)
 		/* XXX: Atomic assignment, needs volatile/membar ? */
 		bo->state = BOS_FINISHED;
 	}
-	if (obj->objcore != NULL)
+	if (obj->objcore->objhead != NULL)
 		HSH_Complete(obj->objcore);
 	bo->stats = NULL;
 	VBO_DerefBusyObj(wrk, &bo);
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 6b84bab..bc94352 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -66,23 +66,31 @@
 static const struct hash_slinger *hash;
 
 /*---------------------------------------------------------------------*/
+
+struct objcore *
+HSH_NewObjCore(struct worker *wrk)
+{
+	struct objcore *oc;
+
+	ALLOC_OBJ(oc, OBJCORE_MAGIC);
+	XXXAN(oc);
+	wrk->stats.n_objectcore++;
+	oc->flags |= OC_F_BUSY;
+	return (oc);
+}
+
+/*---------------------------------------------------------------------*/
 /* Precreate an objhead and object for later use */
 static void
 hsh_prealloc(struct worker *wrk)
 {
 	struct objhead *oh;
-	struct objcore *oc;
 	struct waitinglist *wl;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 
-	if (wrk->nobjcore == NULL) {
-		ALLOC_OBJ(oc, OBJCORE_MAGIC);
-		XXXAN(oc);
-		wrk->nobjcore = oc;
-		wrk->stats.n_objectcore++;
-		oc->flags |= OC_F_BUSY;
-	}
+	if (wrk->nobjcore == NULL)
+		wrk->nobjcore = HSH_NewObjCore(wrk);
 	CHECK_OBJ_NOTNULL(wrk->nobjcore, OBJCORE_MAGIC);
 
 	if (wrk->nobjhead == NULL) {
@@ -663,40 +671,31 @@ HSH_Deref(struct dstat *ds, struct objcore *oc, struct object **oo)
 		oc = o->objcore;
 	}
 
-	if (o != NULL && oc == NULL) {
-		/*
-		 * A pass object with neither objcore nor objhdr reference.
-		 * -> simply free the (Transient) storage
-		 */
-		STV_Freestore(o);
-		STV_free(o->objstore);
-		ds->n_object--;
-		return (0);
-	}
-
 	CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
 
 	oh = oc->objhead;
-	CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
+	if (oh != NULL) {
+		CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
 
-	Lck_Lock(&oh->mtx);
-	assert(oh->refcnt > 0);
-	assert(oc->refcnt > 0);
-	r = --oc->refcnt;
-	if (!r)
-		VTAILQ_REMOVE(&oh->objcs, oc, list);
-	else {
-		/* Must have an object */
-		AN(oc->methods);
-	}
-	if (oh->waitinglist != NULL)
-		hsh_rush(ds, oh);
-	Lck_Unlock(&oh->mtx);
-	if (r != 0)
-		return (r);
+		Lck_Lock(&oh->mtx);
+		assert(oh->refcnt > 0);
+		assert(oc->refcnt > 0);
+		r = --oc->refcnt;
+		if (!r)
+			VTAILQ_REMOVE(&oh->objcs, oc, list);
+		else {
+			/* Must have an object */
+			AN(oc->methods);
+		}
+		if (oh->waitinglist != NULL)
+			hsh_rush(ds, oh);
+		Lck_Unlock(&oh->mtx);
+		if (r != 0)
+			return (r);
 
-	BAN_DestroyObj(oc);
-	AZ(oc->ban);
+		BAN_DestroyObj(oc);
+		AZ(oc->ban);
+	}
 
 	if (oc->methods != NULL) {
 		oc_freeobj(oc);
@@ -705,11 +704,13 @@ HSH_Deref(struct dstat *ds, struct objcore *oc, struct object **oo)
 	FREE_OBJ(oc);
 
 	ds->n_objectcore--;
-	/* Drop our ref on the objhead */
-	assert(oh->refcnt > 0);
-	if (hash->deref(oh))
-		return (0);
-	HSH_DeleteObjHead(ds, oh);
+	if (oh != NULL) {
+		/* Drop our ref on the objhead */
+		assert(oh->refcnt > 0);
+		if (hash->deref(oh))
+			return (0);
+		HSH_DeleteObjHead(ds, oh);
+	}
 	return (0);
 }
 
diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c
index 2e56666..37fb7a0 100644
--- a/bin/varnishd/cache/cache_vrt_var.c
+++ b/bin/varnishd/cache/cache_vrt_var.c
@@ -141,7 +141,7 @@ VRT_l_beresp_saintmode(const struct sess *sp, double a)
 	if (!vbc->backend)
 		return;
 	CHECK_OBJ_NOTNULL(vbc->backend, BACKEND_MAGIC);
-	if (!sp->req->objcore)
+	if (!sp->req->objcore->objhead)
 		return;
 	CHECK_OBJ_NOTNULL(sp->req->objcore, OBJCORE_MAGIC);
 
diff --git a/bin/varnishd/hash/hash_slinger.h b/bin/varnishd/hash/hash_slinger.h
index 17d67f9..cbf62ba 100644
--- a/bin/varnishd/hash/hash_slinger.h
+++ b/bin/varnishd/hash/hash_slinger.h
@@ -60,6 +60,7 @@ void HSH_AddString(const struct sess *sp, const char *str);
 void HSH_Insert(struct worker *, const void *hash, struct objcore *);
 void HSH_Purge(const struct sess *, struct objhead *, double ttl, double grace);
 void HSH_config(const char *h_arg);
+struct objcore *HSH_NewObjCore(struct worker *wrk);
 
 #ifdef VARNISH_CACHE_CHILD
 
diff --git a/bin/varnishd/storage/stevedore.c b/bin/varnishd/storage/stevedore.c
index f1cbe94..53846a0 100644
--- a/bin/varnishd/storage/stevedore.c
+++ b/bin/varnishd/storage/stevedore.c
@@ -237,6 +237,7 @@ STV_MkObject(struct stevedore *stv, struct busyobj *bo, struct objcore **ocp,
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
 	CHECK_OBJ_NOTNULL(soc, STV_OBJ_SECRETES_MAGIC);
 	AN(ocp);
+	CHECK_OBJ_NOTNULL((*ocp), OBJCORE_MAGIC);
 
 	assert(PAOK(ptr));
 	assert(PAOK(soc->wsl));
@@ -262,17 +263,14 @@ STV_MkObject(struct stevedore *stv, struct busyobj *bo, struct objcore **ocp,
 	VTAILQ_INIT(&o->store);
 	bo->stats->n_object++;
 
-	if (*ocp != NULL) {
-		CHECK_OBJ_NOTNULL((*ocp), OBJCORE_MAGIC);
-
-		o->objcore = *ocp;
-		*ocp = NULL;     /* refcnt follows pointer. */
+	o->objcore = *ocp;
+	*ocp = NULL;     /* refcnt follows pointer. */
+	if (o->objcore->objhead != NULL)
 		BAN_NewObjCore(o->objcore);
 
-		o->objcore->methods = &default_oc_methods;
-		o->objcore->priv = o;
-		o->objcore->priv2 = (uintptr_t)stv;
-	}
+	o->objcore->methods = &default_oc_methods;
+	o->objcore->priv = o;
+	o->objcore->priv2 = (uintptr_t)stv;
 	return (o);
 }
 
@@ -356,8 +354,11 @@ STV_NewObject(struct busyobj *bo, struct objcore **ocp, const char *hint,
 		}
 	}
 
-	if (o == NULL)
+	if (o == NULL) {
+		AN(*ocp);
 		return (NULL);
+	}
+	AZ(*ocp);
 	CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC);
 	CHECK_OBJ_NOTNULL(o->objstore, STORAGE_MAGIC);
 	return (o);



More information about the varnish-commit mailing list