A LOT more H2 plumbing.

bin/varnishd/http2/cache_http2.h

@@ -221,6 +221,8 @@ h2_error H2_Send(struct worker *, const struct h2_req *,
 struct h2_req * h2_new_req(const struct worker *, struct h2_sess *,
     unsigned stream, struct req *);
 void h2_del_req(struct worker *, struct h2_req *);
+void h2_kill_req(struct worker *, const struct h2_sess *,
+    struct h2_req *, h2_error);
 int h2_rxframe(struct worker *, struct h2_sess *);
 h2_error h2_set_setting(struct h2_sess *, const uint8_t *);
 void h2_req_body(struct req*);

bin/varnishd/http2/cache_http2_deliver.c

@@ -260,7 +260,6 @@ h2_deliver(struct req *req, struct boc *boc, int sendbody)
 
 	WS_Release(req->ws, 0);
 
-
 	if (sendbody) {
 		VDP_push(req, h2_bytes, NULL, 1, "H2");
 		err = VDP_DeliverObj(req);

bin/varnishd/http2/cache_http2_proto.c

@@ -190,6 +190,31 @@ h2_del_req(struct worker *wrk, struct h2_req *r2)
 	SES_Delete(sp, SC_RX_JUNK, NAN);
 }
 
+void
+h2_kill_req(struct worker *wrk, const struct h2_sess *h2,
+    struct h2_req *r2, h2_error h2e)
+{
+
+	VSLb(h2->vsl, SLT_Debug, "KILL st=%u state=%d", r2->stream, r2->state);
+	Lck_Lock(&h2->sess->mtx);
+	r2->error = h2e;
+	switch (r2->state) {
+	case H2_S_CLOS_REM:
+		if (r2->wrk != NULL)
+			AZ(pthread_cond_signal(h2->cond));
+		r2 = NULL;
+		break;
+	case H2_S_OPEN:
+		(void)h2h_decode_fini(h2, r2->decode);
+		break;
+	default:
+		break;
+	}
+	Lck_Unlock(&h2->sess->mtx);
+	if (r2 != NULL)
+		h2_del_req(wrk, r2);
+}
+
 /**********************************************************************/
 
 static void
@@ -241,8 +266,7 @@ h2_rx_ping(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 	(void)r2;
 	if (h2->rxf_len != 8)				// rfc7540,l,2364,2366
 		return (H2CE_FRAME_SIZE_ERROR);
-	if (h2->rxf_stream != 0)			// rfc7540,l,2359,2362
-		return (H2CE_PROTOCOL_ERROR);
+	AZ(h2->rxf_stream);				// rfc7540,l,2359,2362
 	if (h2->rxf_flags != 0)				// We never send pings
 		return (H2SE_PROTOCOL_ERROR);
 	H2_Send_Get(wrk, h2, r2);
@@ -275,20 +299,9 @@ h2_rx_rst_stream(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 
 	if (h2->rxf_len != 4)			// rfc7540,l,2003,2004
 		return (H2CE_FRAME_SIZE_ERROR);
-	if (r2 == NULL)
-		return (0);
-	Lck_Lock(&h2->sess->mtx);
-	r2->error = h2_streamerror(vbe32dec(h2->rxf_data));
-VSLb(h2->vsl, SLT_Debug, "H2RST %u %d %s", r2->stream, r2->state, r2->error->name);
-	if (r2->wrk != NULL)
-		AZ(pthread_cond_signal(h2->cond));
-	if (r2->state != H2_S_IDLE) {
-		r2->state = H2_S_CLOSED;
-		r2 = NULL;
-	}
-	Lck_Unlock(&h2->sess->mtx);
 	if (r2 != NULL)
-		h2_del_req(wrk, r2);
+		h2_kill_req(wrk, h2, r2,
+		    h2_streamerror(vbe32dec(h2->rxf_data)));
 	return (0);
 }
 
@@ -463,7 +476,6 @@ h2_end_headers(const struct worker *wrk, const struct h2_sess *h2,
 	h2_error h2e;
 
 	assert(r2->state == H2_S_OPEN);
-	r2->state = H2_S_CLOS_REM;
 	h2e = h2h_decode_fini(h2, r2->decode);
 	FREE_OBJ(r2->decode);
 	if (h2e != NULL) {
@@ -480,6 +492,7 @@ h2_end_headers(const struct worker *wrk, const struct h2_sess *h2,
 	req->req_step = R_STP_TRANSPORT;
 	req->task.func = h2_do_req;
 	req->task.priv = req;
+	r2->state = H2_S_CLOS_REM;		// XXX: not _quite_ true
 	XXXAZ(Pool_Task(wrk->pool, &req->task, TASK_QUEUE_REQ));
 	return (0);
 }
@@ -535,7 +548,10 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 	}
 	h2e = h2h_decode_bytes(h2, r2->decode, p, l);
 	if (h2e != NULL) {
-		VSL(SLT_Debug, 0, "H2H_DECODE_BYTES %s", h2e->name);
+		VSL(SLT_Debug, 0, "H2H_DECODE_BYTES(hdr) %s", h2e->name);
+		(void)h2h_decode_fini(h2, r2->decode);
+		AZ(r2->req->ws->r);
+		h2_del_req(wrk, r2);
 		return (h2e);
 	}
 	if (h2->rxf_flags & H2FF_HEADERS_END_HEADERS)
@@ -558,7 +574,10 @@ h2_rx_continuation(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 	req = r2->req;
 	h2e = h2h_decode_bytes(h2, r2->decode, h2->rxf_data, h2->rxf_len);
 	if (h2e != NULL) {
-		VSL(SLT_Debug, 0, "H2H_DECODE_BYTES %s", h2e->name);
+		VSL(SLT_Debug, 0, "H2H_DECODE_BYTES(cont) %s", h2e->name);
+		(void)h2h_decode_fini(h2, r2->decode);
+		AZ(r2->req->ws->r);
+		h2_del_req(wrk, r2);
 		return (h2e);
 	}
 	if (h2->rxf_flags & H2FF_HEADERS_END_HEADERS)
@@ -677,6 +696,7 @@ h2_req_fail(struct req *req, enum sess_close reason)
 {
 	assert(reason > 0);
 	assert(req->sp->fd != 0);
+	VSLb(req->vsl, SLT_Debug, "H2FAILREQ");
 }
 
 /**********************************************************************/
@@ -836,7 +856,7 @@ h2_rxframe(struct worker *wrk, struct h2_sess *h2)
 		h2->bogosity++;
 		VSLb(h2->vsl, SLT_Debug,
 		    "H2: Unknown flags 0x%02x on %s (ignored)",
-		    (uint8_t)h2->rxf_flags, h2f->name);
+		    (uint8_t)h2->rxf_flags & ~h2f->flags, h2f->name);
 		h2->rxf_flags &= h2f->flags;
 	}
 

bin/varnishd/http2/cache_http2_session.c

@@ -201,6 +201,10 @@ h2_ou_session(struct worker *wrk, struct h2_sess *h2,
 
 	if (h2_b64url_settings(h2, req)) {
 		VSLb(h2->vsl, SLT_Debug, "H2: Bad HTTP-Settings");
+		AN (req->vcl);
+		VCL_Rel(&req->vcl);
+		CNT_AcctLogCharge(wrk->stats, req);
+		Req_Release(req);
 		return (0);
 	}
 
@@ -317,20 +321,25 @@ h2_new_session(struct worker *wrk, void *arg)
 	VTAILQ_FOREACH(r2, &h2->streams, list) {
 		if (r2->error == 0)
 			r2->error = h2->error;
+#if 0
 		if (r2->wrk != NULL)
-			AZ(pthread_cond_signal(&r2->wrk->cond));
+			AZ(pthread_cond_signal(&r2->wrk->cond)); // XXX Why?
+#endif
 	}
+	AZ(pthread_cond_signal(h2->cond));
 	while (1) {
 		VTAILQ_FOREACH(r2, &h2->streams, list)
-			if (r2->state == H2_S_IDLE && r2 != h2->req0)
+			if (r2->state != H2_S_CLOS_REM && r2 != h2->req0)
 				break;
 		if (r2 == NULL)
 			break;
 		Lck_Unlock(&sp->mtx);
-		h2_del_req(wrk, r2);
+		h2_kill_req(wrk, h2, r2, h2->error);
 		Lck_Lock(&sp->mtx);
 	}
 	VSLb(h2->vsl, SLT_Debug, "H2CLEAN done");
+	VTAILQ_FOREACH(r2, &h2->streams, list)
+		VSLb(h2->vsl, SLT_Debug, "ST %u %d", r2->stream, r2->state);
 	Lck_Unlock(&sp->mtx);
 	h2->cond = NULL;
 	h2_del_req(wrk, h2->req0);

bin/varnishtest/tests/t02001.vtc

@@ -39,7 +39,12 @@ server s1 {
 	txresp -status 402 -bodylen 11
 } -start
 
-delay .5
+varnish v1 -vsl_catchup
+
+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
 
 varnish v1 -cliok "param.set feature +http2"
 
@@ -58,7 +63,12 @@ client c1 {
 	expect resp.bodylen == 8
 } -run
 
-delay .5
+varnish v1 -vsl_catchup
+
+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
 
 client c1 {
 	send "GET /upgrade2 HTTP/1.1\r\n"
@@ -86,6 +96,13 @@ client c1 {
 	} -run
 } -run
 
+varnish v1 -vsl_catchup
+
+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
+
 client c1 {
 	# Illegal HTTP2-Settings
 	send "GET /noupgrade HTTP/1.1\r\n"
@@ -96,6 +113,13 @@ client c1 {
 	expect_close
 } -run
 
+varnish v1 -vsl_catchup
+
+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
+
 client c1 {
 	# PRISM with error in last bit
 	send "GET /noupgrade HTTP/1.1\r\n"
@@ -111,8 +135,7 @@ client c1 {
 	expect_close
 } -run
 
-# XXX: Tests temporarily neutered, they are too flakey
-#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
+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

bin/varnishtest/tests/t02003.vtc

-varnishtest "H2 Stream error conditions"
+varnishtest "H2 Frame coverage/error conditions"
 
 server s1 {
 	rxreq
@@ -26,6 +26,13 @@ client c1 {
 	stream 0 -wait
 } -run
 
+varnish v1 -vsl_catchup
+
+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
+
 #######################################################################
 # Test reverse order stream numbers
 
@@ -44,6 +51,13 @@ client c1 {
 	stream 0 -wait
 } -run
 
+varnish v1 -vsl_catchup
+
+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
+
 #######################################################################
 # Test WINDOW_UPDATE error conditions
 
@@ -68,7 +82,13 @@ client c1 {
 	} -start
 	stream 5 {
 		txprio
-		sendhex "000003 08 00 00000005 010203"
+		sendhex "000003 08 00 00000005"
+		delay .1
+		sendhex 01
+		delay .1
+		sendhex 02
+		delay .1
+		sendhex 03
 	} -run
 	stream 0 -wait
 } -run
@@ -93,6 +113,13 @@ client c1 {
 	} -run
 } -run
 
+varnish v1 -vsl_catchup
+
+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
+
 #######################################################################
 # Test PING error conditions
 
@@ -114,6 +141,22 @@ client c1 {
 	} -run
 } -run
 
+client c1 {
+	stream 0 {
+		sendhex "000007 06 80 00000000 01020304050607"
+		rxgoaway
+		expect goaway.laststream == 0
+		expect goaway.err == FRAME_SIZE_ERROR
+	} -run
+} -run
+
+varnish v1 -vsl_catchup
+
+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
+
 #######################################################################
 # Test PUSH_PROMISE error conditions
 
@@ -130,6 +173,14 @@ client c1 {
 	stream 0 -wait
 } -run
 
+varnish v1 -vsl_catchup
+
+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
+
+#######################################################################
 # Test RST_STREAM error conditions
 
 client c1 {
@@ -166,6 +217,20 @@ client c1 {
 	} -run
 } -run
 
+client c1 {
+	stream 1 {
+		txreq -nohdrend
+		txrst -err 0x666
+	} -run
+} -run
+
+varnish v1 -vsl_catchup
+
+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
+
 #######################################################################
 # Test SETTING error conditions
 
@@ -186,8 +251,7 @@ client c1 {
 		rxgoaway
 		expect goaway.err == PROTOCOL_ERROR
 		expect goaway.laststream == 0
-	} -run
-} -run
+	} -run } -run
 
 client c1 {
 	stream 0 {
@@ -207,3 +271,112 @@ client c1 {
 		rxping
 	} -run
 } -run
+
+varnish v1 -vsl_catchup
+
+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
+
+#######################################################################
+# Test GOAWAY error conditions
+
+client c1 {
+	stream 0 {
+		txgoaway -err 2
+	} -run
+	expect_close
+} -run
+
+client c1 {
+	stream 0 {
+		txgoaway -err 2222
+	} -run
+	expect_close
+} -run
+
+
+varnish v1 -vsl_catchup
+
+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
+
+#######################################################################
+# Test HEADERS error conditions
+
+client c1 {
+	stream 1 {
+		txreq -nostrend
+		txreq -nostrend
+	} -run
+	stream 0 {
+		rxgoaway
+	} -run
+	expect_close
+} -run
+
+client c1 {
+	stream 0 {
+		sendhex 00000c
+		sendhex 01
+		sendhex 05
+		sendhex 00000001
+		sendhex ffffffff
+		sendhex ffffffff
+		sendhex ffffffff
+		rxgoaway
+		expect goaway.err == COMPRESSION_ERROR
+	} -run
+} -run
+
+varnish v1 -vsl_catchup
+
+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
+
+#######################################################################
+# Test CONTINUATION error conditions
+
+client c1 {
+	stream 1 {
+		txreq -nostrend
+		txcont -hdr "foo" "bar"
+	} -run
+	stream 0 {
+		rxgoaway
+	} -run
+	expect_close
+} -run
+
+client c1 {
+	stream 0 {
+		sendhex 000014
+		sendhex 01
+		sendhex 01
+		sendhex 00000001
+		sendhex {8286 8441 0f77 7777 2e65 7861 6d70 6c65 2e63 6f6d}
+
+		sendhex 00000c
+		sendhex 09
+		sendhex 04
+		sendhex 00000001
+		sendhex ffffffff
+		sendhex ffffffff
+		sendhex ffffffff
+		rxgoaway
+		expect goaway.err == COMPRESSION_ERROR
+	} -run
+} -run
+
+varnish v1 -vsl_catchup
+
+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
+

bin/varnishtest/tests/t02008.vtc

@@ -27,7 +27,7 @@ client c1 {
 		txprio
 	} -run
 	stream 0 {
-		txgoaway -err 7
+		txgoaway -err 2
 	} -run
 	expect_close
 } -run