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

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);
 
 

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 {

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"))