[master] b2b6e32 Fix a race spotted by Nils: We have to hold a ref to the busyobj if we want to examine it in any detail.

[Poul-Henning Kamp]

commit b2b6e329da78e465720bec9a26a905e8a77cab92
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Jun 30 19:34:47 2014 +0000

    Fix a race spotted by Nils:  We have to hold a ref to the
    busyobj if we want to examine it in any detail.
    
    Hold that ref over all of V1D_Deliver() (different from Nils
    patch) to make sure we have a consistent view on streaming
    vs. no streaming throughout.
    
    Also use the bo to clean up a hack related to ESI includes.
    
    Diagnosed by:	Nils Goroll

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index ad4c3ab..038510d 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -818,7 +818,7 @@ HTTP1_Chunked(struct http_conn *htc, intptr_t *priv, const char **error,
 
 /* cache_http1_deliver.c */
 unsigned V1D_FlushReleaseAcct(struct req *req);
-void V1D_Deliver(struct req *);
+void V1D_Deliver(struct req *, struct busyobj *);
 void V1D_Deliver_Synth(struct req *req);
 
 
diff --git a/bin/varnishd/cache/cache_http1_deliver.c b/bin/varnishd/cache/cache_http1_deliver.c
index 1bdcef7..73aea69 100644
--- a/bin/varnishd/cache/cache_http1_deliver.c
+++ b/bin/varnishd/cache/cache_http1_deliver.c
@@ -87,7 +87,7 @@ v1d_range_bytes(struct req *req, enum vdp_action act, const void *ptr,
 /*--------------------------------------------------------------------*/
 
 static void
-v1d_dorange(struct req *req, const char *r)
+v1d_dorange(struct req *req, struct busyobj *bo, const char *r)
 {
 	ssize_t len, low, high, has_low;
 
@@ -96,8 +96,8 @@ v1d_dorange(struct req *req, const char *r)
 	assert(http_GetStatus(req->obj->http) == 200);
 
 	/* We must snapshot the length if we're streaming from the backend */
-	if (req->obj->objcore->busyobj != NULL)
-		len = VBO_waitlen(req->obj->objcore->busyobj, -1);
+	if (bo != NULL)
+		len = VBO_waitlen(bo, -1);
 	else
 		len = req->obj->len;
 
@@ -228,10 +228,11 @@ V1D_FlushReleaseAcct(struct req *req)
 }
 
 void
-V1D_Deliver(struct req *req)
+V1D_Deliver(struct req *req, struct busyobj *bo)
 {
 	char *r;
 	enum objiter_status ois;
+	ssize_t l;
 
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	CHECK_OBJ_NOTNULL(req->obj, OBJECT_MAGIC);
@@ -247,7 +248,7 @@ V1D_Deliver(struct req *req)
 		req->res_mode &= ~RES_LEN;
 		http_Unset(req->resp, H_Content_Length);
 		req->wantbody = 0;
-	} else if (req->obj->objcore->busyobj == NULL) {
+	} else if (bo == NULL) {
 		/* XXX: Not happy with this convoluted test */
 		req->res_mode |= RES_LEN;
 		if (!(req->obj->objcore->flags & OC_F_PASS) ||
@@ -310,7 +311,7 @@ V1D_Deliver(struct req *req)
 	    http_GetStatus(req->resp) == 200) {
 		http_SetHeader(req->resp, "Accept-Ranges: bytes");
 		if (http_GetHdr(req->http, H_Range, &r))
-			v1d_dorange(req, r);
+			v1d_dorange(req, bo, r);
 	}
 
 	if (req->res_mode & RES_ESI)
@@ -334,8 +335,11 @@ V1D_Deliver(struct req *req)
 	} else if (req->res_mode & RES_ESI) {
 		ESI_Deliver(req);
 	} else if (req->res_mode & RES_ESI_CHILD && req->gzip_resp) {
-		while (req->obj->objcore->busyobj)
-			(void)usleep(10000);
+		l = -1;
+		while (req->obj->objcore->busyobj) {
+			assert(bo != NULL);
+			l = VBO_waitlen(bo, l);
+		}
 		ESI_DeliverChild(req);
 	} else if (req->res_mode & RES_GUNZIP ||
 	    (req->res_mode & RES_ESI_CHILD &&
@@ -421,7 +425,7 @@ V1D_Deliver_Synth(struct req *req)
 	    http_GetStatus(req->resp) == 200) {
 		http_SetHeader(req->resp, "Accept-Ranges: bytes");
 		if (http_GetHdr(req->http, H_Range, &r))
-			v1d_dorange(req, r);
+			v1d_dorange(req, NULL, r);
 	}
 
 	WRW_Reserve(req->wrk, &req->sp->fd, req->vsl, req->t_prev);
@@ -441,8 +445,6 @@ V1D_Deliver_Synth(struct req *req)
 #if 0
 	XXX: Missing pretend GZIP for esi-children
 	} else if (req->res_mode & RES_ESI_CHILD && req->gzip_resp) {
-		while (req->obj->objcore->busyobj)
-			(void)usleep(10000);
 		ESI_DeliverChild(req);
 #endif
 	} else {
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index f987568..161c5e8 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -87,6 +87,7 @@ DOT deliver:deliver:s -> DONE [style=bold,color=blue]
 static enum req_fsm_nxt
 cnt_deliver(struct worker *wrk, struct req *req)
 {
+	struct busyobj *bo;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
@@ -154,7 +155,12 @@ cnt_deliver(struct worker *wrk, struct req *req)
 		req->wantbody = 0;
 	}
 
-	V1D_Deliver(req);
+	/* Grab a ref to the bo if there is one, and hand it down */
+	bo = HSH_RefBusy(req->obj->objcore);
+	V1D_Deliver(req, bo);
+	if (bo != NULL)
+		VBO_DerefBusyObj(req->wrk, &bo);
+
 	VSLb_ts_req(req, "Resp", W_TIM_real(wrk));
 
 	if (http_HdrIs(req->resp, H_Connection, "close"))