[master] 0cfa869 Don't HTC_RxStuff with a non-reserved workspace

[Dag Haavi Finstad]

commit 0cfa8698eedecb92047316039dbb3e1733239a76
Author: Dag Haavi Finstad <daghf at varnish-software.com>
Date:   Thu Jan 18 16:35:38 2018 +0100

    Don't HTC_RxStuff with a non-reserved workspace
    
    This commit partially reverts and reworks b9f7170b0. We now do the "wait
    for active streams" handling via the loop in h2_new_session, to ensure
    the workspace is in a predictable state once HTC_RxStuff is called
    again.
    
    Fixes: #2539

diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 89eb620..45ab188 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -891,40 +891,36 @@ h2_rxframe(struct worker *wrk, struct h2_sess *h2)
 	enum htc_status_e hs;
 	h2_frame h2f;
 	h2_error h2e;
-	int again;
 	struct h2_req *r2, *r22;
 	char b[8];
 
 	ASSERT_RXTHR(h2);
 	(void)VTCP_blocking(*h2->htc->rfd);
-	while (1) {
-		h2->sess->t_idle = VTIM_real();
-		hs = HTC_RxStuff(h2->htc, h2_frame_complete,
-		    NULL, NULL, NAN,
-		    h2->sess->t_idle + cache_param->timeout_idle,
-		    16384 + 9);		// rfc7540,l,4228,4228
-		if (hs == HTC_S_COMPLETE)
-			break;
-		else if (hs == HTC_S_TIMEOUT) {
-			again = 0;
-			VTAILQ_FOREACH_SAFE(r2, &h2->streams, list, r22) {
-				switch (r2->state) {
-				case H2_S_CLOSED:
-					if (!r2->scheduled)
-						h2_del_req(wrk, r2);
-					break;
-				case H2_S_OPEN:
-				case H2_S_CLOS_REM:
-				case H2_S_CLOS_LOC:
-					again = 1;
-					break;
-				default:
-					break;
-				}
+	h2->sess->t_idle = VTIM_real();
+	hs = HTC_RxStuff(h2->htc, h2_frame_complete,
+	    NULL, NULL, NAN,
+	    h2->sess->t_idle + cache_param->timeout_idle,
+	    16384 + 9);		// rfc7540,l,4228,4228
+	switch (hs) {
+	case HTC_S_COMPLETE:
+		break;
+	case HTC_S_TIMEOUT:
+		VTAILQ_FOREACH_SAFE(r2, &h2->streams, list, r22) {
+			switch (r2->state) {
+			case H2_S_CLOSED:
+				if (!r2->scheduled)
+					h2_del_req(wrk, r2);
+				break;
+			case H2_S_OPEN:
+			case H2_S_CLOS_REM:
+			case H2_S_CLOS_LOC:
+				return (1);
+			default:
+				break;
 			}
-			if (again)
-				continue;
 		}
+		/* FALLTHROUGH */
+	default:
 		Lck_Lock(&h2->sess->mtx);
 		VSLb(h2->vsl, SLT_Debug, "H2: No frame (hs=%d)", hs);
 		h2->error = H2CE_NO_ERROR;
diff --git a/bin/varnishtest/tests/r02539.vtc b/bin/varnishtest/tests/r02539.vtc
new file mode 100644
index 0000000..de7be73
--- /dev/null
+++ b/bin/varnishtest/tests/r02539.vtc
@@ -0,0 +1,31 @@
+varnishtest "2359: H/2 Avoid RxStuff with a non-reserved WS"
+
+barrier b1 sock 2
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -cliok "param.set feature +http2"
+varnish v1 -cliok "param.set timeout_idle 1"
+
+varnish v1 -vcl+backend {
+	import vtc;
+
+	sub vcl_deliver {
+		vtc.barrier_sync("${b1_sock}");
+	}
+} -start
+
+client c1 {
+	stream 1 {
+		txreq
+		rxresp
+		expect resp.status == 200
+	} -start
+
+	delay 3
+	barrier b1 sync
+	stream 1 -wait
+} -run