[master] 8317ebe Refuse streams that can't be scheduled

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Sep 4 10:18:06 CEST 2017


commit 8317ebe30b7b59ddb5ca2835cda506ca0b2a9787
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Aug 22 18:02:17 2017 +0200

    Refuse streams that can't be scheduled
    
    A client can safely retry the request because no application logic is
    involved when no worker can enter the VCL state machine. At this point
    the HPACK block was properly processed, so Varnish remains in sync with
    the client and may process future streams.
    
    Fixes #2311

diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 40a5a9b..122e61f 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -530,7 +530,8 @@ h2_end_headers(struct worker *wrk, const struct h2_sess *h2,
 	req->task.func = h2_do_req;
 	req->task.priv = req;
 	r2->scheduled = 1;
-	XXXAZ(Pool_Task(wrk->pool, &req->task, TASK_QUEUE_REQ));
+	if (Pool_Task(wrk->pool, &req->task, TASK_QUEUE_REQ) != 0)
+		return (H2SE_REFUSED_STREAM); //rfc7540,l,3326,3329
 	return (0);
 }
 
diff --git a/bin/varnishtest/tests/t02011.vtc b/bin/varnishtest/tests/t02011.vtc
new file mode 100644
index 0000000..c906e92
--- /dev/null
+++ b/bin/varnishtest/tests/t02011.vtc
@@ -0,0 +1,68 @@
+varnishtest "Can't schedule stream"
+
+barrier b1 sock 3
+barrier b2 sock 3
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+# We need to confirm that a refused stream doesn't harm other streams, so we
+# want a round trip to the backend to ensure that. This requires 3 threads to
+# be busy simultaneously before a stream can be refused:
+#
+# - one for c1's session
+# - one for stream 1
+# - one for the backend request
+#
+# To work around the reserve's default logic, an additional thread can be
+# created, but won't be consumed for stream 3 because the pool will reserve
+# it for a backend transaction. Otherwise it could starve the pool and dead
+# lock v1.
+
+varnish v1 -cliok "param.set thread_pools 1"
+varnish v1 -cliok "param.set thread_pool_min 4"
+varnish v1 -cliok "param.set thread_pool_max 4"
+varnish v1 -cliok "param.set thread_pool_reserve 1"
+varnish v1 -cliok "param.set thread_queue_limit 0"
+varnish v1 -cliok "param.set feature +http2"
+varnish v1 -cliok "param.set debug +syncvsl"
+
+varnish v1 -vcl+backend {
+	import vtc;
+
+	sub vcl_recv {
+		if (req.http.should == "reset") {
+			return (fail);
+		}
+	}
+
+	sub vcl_backend_fetch {
+		vtc.barrier_sync("${b1_sock}");
+		vtc.barrier_sync("${b2_sock}");
+	}
+} -start
+
+client c1 {
+	txpri
+	stream 0 rxsettings -run
+
+	stream 1 {
+		txreq -hdr should sync
+		barrier b1 sync
+		barrier b2 sync
+		rxresp
+		expect resp.status == 200
+	} -start
+
+	barrier b1 sync
+
+	stream 3 {
+		txreq -hdr should reset
+		rxrst
+		expect rst.err == REFUSED_STREAM
+	} -run
+
+	barrier b2 sync
+} -run



More information about the varnish-commit mailing list