[master] 8a50626 Correctly clean up H2 sessions

Poul-Henning Kamp phk at FreeBSD.org
Tue Sep 13 09:52:14 CEST 2016


commit 8a506262f14ca7df8a833bfcf4e318a15af7aea6
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Tue Sep 13 07:50:38 2016 +0000

    Correctly clean up H2 sessions

diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 62b5079..73d0c95 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -141,7 +141,6 @@ h2_new_sess(const struct worker *wrk, struct sess *sp, struct req *srq)
 		h2 = WS_Alloc(srq->ws, sizeof *h2);
 		AN(h2);
 		INIT_OBJ(h2, H2_SESS_MAGIC);
-		h2->refcnt = 1;
 		h2->srq = srq;
 		h2->htc = srq->htc;
 		h2->ws = srq->ws;
@@ -203,20 +202,29 @@ static void
 h2_del_req(struct worker *wrk, struct h2_req *r2, enum h2_error_e err)
 {
 	struct h2_sess *h2;
+	struct sess *sp;
+	struct req *req;
 	int r;
 
 	(void)err;
 	h2 = r2->h2sess;
-	Lck_Lock(&h2->sess->mtx);
+	sp = h2->sess;
+	Lck_Lock(&sp->mtx);
 	assert(h2->refcnt > 0);
 	r = --h2->refcnt;
 	/* XXX: PRIORITY reshuffle */
 	VTAILQ_REMOVE(&h2->streams, r2, list);
-	Lck_Unlock(&h2->sess->mtx);
-	Req_Cleanup(h2->sess, wrk, r2->req);
+	Lck_Unlock(&sp->mtx);
+	Req_Cleanup(sp, wrk, r2->req);
+	Req_Release(r2->req);
 	if (r)
 		return;
-	SES_Delete(h2->sess, SC_RX_JUNK, NAN);
+
+	/* All streams gone, including stream #0, clean up */
+	req = h2->srq;
+	Req_Cleanup(sp, wrk, req);
+	Req_Release(req);
+	SES_Delete(sp, SC_RX_JUNK, NAN);
 }
 
 /**********************************************************************
@@ -687,7 +695,7 @@ h2_new_session(struct worker *wrk, void *arg)
 	struct req *req;
 	struct sess *sp;
 	struct h2_sess *h2;
-	struct h2_req *r2;
+	struct h2_req *r2, *r22;
 	char *wsp;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
@@ -742,10 +750,11 @@ h2_new_session(struct worker *wrk, void *arg)
 		HTC_RxInit(h2->htc, wrk->aws);
 	}
 
-	r2 = VTAILQ_FIRST(&h2->streams);
-	CHECK_OBJ_NOTNULL(r2, H2_REQ_MAGIC);
-	assert(r2->stream == 0);
-	h2_del_req(wrk, r2, H2E_NO_ERROR);
+	/* Delete all idle streams */
+	VTAILQ_FOREACH_SAFE(r2, &h2->streams, list, r22) {
+		if (r2->state == H2_S_IDLE)
+			h2_del_req(wrk, r2, H2E_NO_ERROR);
+	}
 }
 
 struct transport H2_transport = {
diff --git a/bin/varnishtest/tests/t02000.vtc b/bin/varnishtest/tests/t02000.vtc
index da3d6b2..7ebf4b3 100644
--- a/bin/varnishtest/tests/t02000.vtc
+++ b/bin/varnishtest/tests/t02000.vtc
@@ -32,4 +32,10 @@ client c1 {
 		expect ping.ack == "true"
 		expect ping.data == "_-__-_-_"
 	} -run
+	stream 7 -wait
 } -run
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
diff --git a/bin/varnishtest/tests/t02001.vtc b/bin/varnishtest/tests/t02001.vtc
index a7fa8b0..43601a7 100644
--- a/bin/varnishtest/tests/t02001.vtc
+++ b/bin/varnishtest/tests/t02001.vtc
@@ -60,3 +60,8 @@ client c1 {
 		expect resp.bodylen == 11
 	} -run
 } -run
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
diff --git a/bin/varnishtest/tests/t02002.vtc b/bin/varnishtest/tests/t02002.vtc
index ebb0146..f2cfb50 100644
--- a/bin/varnishtest/tests/t02002.vtc
+++ b/bin/varnishtest/tests/t02002.vtc
@@ -35,3 +35,8 @@ client c1 {
 		expect resp.bodylen == 20
 	} -run
 } -run
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0



More information about the varnish-commit mailing list