[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
phk at FreeBSD.org
Mon Dec 16 14:31:14 CET 2013
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
More information about the varnish-commit
mailing list