[master] b253bf9 When a streaming delivery of a pass object is terminated prematurely, we cannot just throw the storage away, we have to wait for the fetch-thread to go away, possibly in response to a new "abandon" signal.

[Poul-Henning Kamp]

commit b253bf9c52929e13f13c65670dd08871feb1f977
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Dec 16 13:28:47 2013 +0000

    When a streaming delivery of a pass object is terminated prematurely,
    we cannot just throw the storage away, we have to wait for the
    fetch-thread to go away, possibly in response to a new "abandon"
    signal.
    
    Spotted first by:	scoof
    Fixes	#1391

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 38ca5e3..5998e38 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -573,6 +573,7 @@ struct busyobj {
 	/* do_pass is our intent, uncacheable is the result */
 	unsigned		do_pass;
 	unsigned		uncacheable;
+	unsigned		abandon;
 
 	/* Timeouts */
 	double			connect_timeout;
diff --git a/bin/varnishd/cache/cache_fetch_proc.c b/bin/varnishd/cache/cache_fetch_proc.c
index f080bfd..006768d 100644
--- a/bin/varnishd/cache/cache_fetch_proc.c
+++ b/bin/varnishd/cache/cache_fetch_proc.c
@@ -201,6 +201,19 @@ VFP_Fetch_Body(struct busyobj *bo, ssize_t est)
 	}
 
 	do {
+		if (bo->abandon) {
+			/*
+			 * A pass object and delivery was terminted
+			 * We don't fail the fetch, in order for hit-for-pass
+			 * objects to be created.
+			 */
+			AN(bo->fetch_objcore->flags & OC_F_PASS);
+			VSLb(bo->vsl, SLT_FetchError,
+			    "Pass delivery abandonned");
+			vfps = VFP_END;
+			bo->should_close = 1;
+			break;
+		}
 		assert(bo->state != BOS_FAILED);
 		if (st == NULL) {
 			st = VFP_GetStorage(bo, est);
diff --git a/bin/varnishd/cache/cache_http1_deliver.c b/bin/varnishd/cache/cache_http1_deliver.c
index dc78078..1419655 100644
--- a/bin/varnishd/cache/cache_http1_deliver.c
+++ b/bin/varnishd/cache/cache_http1_deliver.c
@@ -170,11 +170,13 @@ v1d_WriteDirObj(struct req *req)
 
 	while (1) {
 		ois = ObjIter(oi, &ptr, &len);
-		if (ois == OIS_DATA && !VDP_bytes(req, VDP_NULL,  ptr, len))
-			continue;
-		if (ois == OIS_STREAM && !VDP_bytes(req, VDP_FLUSH,  ptr, len))
-			continue;
-		break;
+		if (ois == OIS_DONE) {
+			AZ(len);
+			break;
+		}
+		if (VDP_bytes(req,
+		     ois == OIS_DATA ? VDP_NULL : VDP_FLUSH,  ptr, len))
+			break;
 	}
 	(void)VDP_bytes(req, VDP_FINISH,  NULL, 0);
 	ObjIterEnd(&oi);
diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c
index c59ebf9..3c2f670 100644
--- a/bin/varnishd/cache/cache_obj.c
+++ b/bin/varnishd/cache/cache_obj.c
@@ -126,8 +126,11 @@ ObjIterEnd(struct objiter **oi)
 	AN(oi);
 	CHECK_OBJ_NOTNULL((*oi), OBJITER_MAGIC);
 	CHECK_OBJ_NOTNULL((*oi)->obj, OBJECT_MAGIC);
-	if ((*oi)->bo != NULL)
+	if ((*oi)->bo != NULL) {
+		if ((*oi)->obj->objcore->flags & OC_F_PASS)
+			(*oi)->bo->abandon = 1;
 		VBO_DerefBusyObj((*oi)->wrk, &(*oi)->bo);
+	}
 	FREE_OBJ((*oi));
 	*oi = NULL;
 }
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index af8531f..adeb37e 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -159,9 +159,16 @@ cnt_deliver(struct worker *wrk, struct req *req)
 
 	V1D_Deliver(req);
 
-	/* No point in saving the body if it is hit-for-pass */
-	if (req->obj->objcore->flags & OC_F_PASS)
+	if (req->obj->objcore->flags & OC_F_PASS) {
+		/*
+		 * No point in saving the body if it is hit-for-pass,
+		 * but we can't yank it until the fetching thread has
+		 * finished/abandonned also.
+		 */
+		while (req->obj->objcore->busyobj != NULL)
+			(void)usleep(100000);
 		STV_Freestore(req->obj);
+	}
 
 	assert(WRW_IsReleased(wrk));
 VSLb(req->vsl, SLT_Debug, "XXX REF %d", req->obj->objcore->refcnt);
diff --git a/bin/varnishtest/tests/r01391.vtc b/bin/varnishtest/tests/r01391.vtc
new file mode 100644
index 0000000..4b98fda
--- /dev/null
+++ b/bin/varnishtest/tests/r01391.vtc
@@ -0,0 +1,39 @@
+varnishtest "client abandoning hit-for-pass"
+
+
+server s1 {
+	rxreq
+	txresp -nolen -hdr "Transfer-Encoding: chunked" -hdr "Set-Cookie: foo=bar"
+	chunked "foo"
+	sema r1 sync 2
+	chunked "bar"
+	delay .1
+	chunkedlen 64
+	delay .1
+	chunkedlen 64
+	chunkedlen 0
+} -start
+
+varnish v1 -vcl+backend {
+} -start
+
+
+client c1 {
+	txreq
+	rxhdrs
+	rxchunk
+	sema r1 sync 2
+} -run
+
+delay 2
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 200
+} -run