[PATCH] Remember to deref our objcore refs on fetch failures and rearm the exp_timer on fetch failure ttl changes.

Martin Blix Grydeland martin at varnish-software.com
Wed May 16 14:05:15 CEST 2012


Fixes: #1138
---
 bin/varnishd/cache/cache_center.c |    4 +-
 bin/varnishd/cache/cache_fetch.c  |    3 +-
 bin/varnishtest/tests/r01138.vtc  |   45 +++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 3 deletions(-)
 create mode 100644 bin/varnishtest/tests/r01138.vtc

diff --git a/bin/varnishd/cache/cache_center.c b/bin/varnishd/cache/cache_center.c
index 332211a..335bda7 100644
--- a/bin/varnishd/cache/cache_center.c
+++ b/bin/varnishd/cache/cache_center.c
@@ -323,7 +323,7 @@ cnt_deliver(struct sess *sp, struct worker *wrk, struct req *req)
 		assert(bo->state >= BOS_FAILED);
 
 		if (bo->state == BOS_FAILED) {
-			req->obj = NULL;
+			HSH_Deref(&wrk->stats, NULL, &req->obj);
 			VBO_DerefBusyObj(wrk, &req->busyobj);
 			req->err_code = 503;
 			sp->step = STP_ERROR;
@@ -901,7 +901,7 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
 		VBO_DerefBusyObj(wrk, &req->busyobj);
 	} else if (bo->state == BOS_FAILED) {
 		/* handle early failures */
-		req->obj = NULL;
+		HSH_Deref(&wrk->stats, NULL, &req->obj);
 		VBO_DerefBusyObj(wrk, &req->busyobj);
 		req->err_code = 503;
 		sp->step = STP_ERROR;
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index ee9147d..7cc1a90 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -647,7 +647,8 @@ FetchBody(struct worker *wrk, void *priv)
 	if (bo->state == BOS_FAILED) {
 		wrk->stats.fetch_failed++;
 		VDI_CloseFd(&bo->vbc);
-		obj->exp.ttl = -1.;
+		EXP_Clr(&obj->exp);
+		EXP_Rearm(obj);
 		obj->len = 0;
 	} else {
 		assert(bo->state == BOS_FETCHING);
diff --git a/bin/varnishtest/tests/r01138.vtc b/bin/varnishtest/tests/r01138.vtc
new file mode 100644
index 0000000..33f0559
--- /dev/null
+++ b/bin/varnishtest/tests/r01138.vtc
@@ -0,0 +1,45 @@
+varnishtest "Leaking objects on fetch failures - #1138"
+
+server s1 {
+	rxreq
+	expect req.url == "/test1"
+	txresp -nolen -hdr "Content-Length: 1024" -bodylen 512
+	close
+
+	accept
+	rxreq
+	expect req.url == "/test2"
+	delay 0.2
+	txresp -nolen -hdr "Content-Length: 1024" -bodylen 512
+} -start
+
+varnish v1 -arg "-p expiry_sleep=0.1 -p default_keep=0 -p default_grace=0" -vcl+backend {
+	sub vcl_fetch {
+		if (req.url ~ "/test1") {
+			# Having no streaming will test error condition
+			# in cnt_fetchbody()
+			set beresp.do_stream = false;
+		} else if (req.url ~ "/test2") {
+			# Doing streaming and slow delivery will test error
+			# condition in cnt_delivery()
+			set beresp.do_stream = true;
+		}
+	}
+} -start
+
+client c1 {
+	txreq -url "/test1"
+	rxresp
+	expect resp.status == 503
+} -run
+
+client c2 {
+	txreq -url "/test2"
+	rxresp
+	expect resp.status == 503
+} -run
+
+# Allow exp_timer to do it's job
+delay 1
+
+varnish v1 -expect n_object == 0
-- 
1.7.4.1




More information about the varnish-dev mailing list