[master] 70d49d5 h2: Fix reembark failure handling

[Dag Haavi Finstad]

commit 70d49d5997503fcc91a693723993fbe06806b56b
Author: Dag Haavi Finstad <daghf at varnish-software.com>
Date:   Wed Mar 14 11:11:38 2018 +0100

    h2: Fix reembark failure handling
    
    Properly get rid of streams that failed to reschedule after being
    waitlisted.
    
    Fixes: #2563
    Fixes: #2592

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index a87e78a..cfe8598 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -232,6 +232,7 @@ void h2_kill_req(struct worker *, const struct h2_sess *,
 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*);
+void h2_cleanup_waiting(struct worker *wrk, struct h2_req *r2);
 task_func_t h2_do_req;
 #ifdef TRANSPORT_MAGIC
 vtr_req_fail_f h2_req_fail;
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 9aa487c..d4ae479 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -37,6 +37,7 @@
 #include "cache/cache_transport.h"
 #include "cache/cache_filter.h"
 #include "http2/cache_http2.h"
+#include "cache/cache_objhead.h"
 
 #include "vend.h"
 #include "vtcp.h"
@@ -946,8 +947,8 @@ h2_stream_tmo(struct h2_sess *h2, const struct h2_req *r2)
 }
 
 /*
- * This is the janitorial task of cleaning up any closed streams, and
- * checking if the session is timed out.
+ * This is the janitorial task of cleaning up any closed & refused
+ * streams, and checking if the session is timed out.
  */
 static int
 h2_sweep(struct worker *wrk, struct h2_sess *h2)
@@ -969,6 +970,13 @@ h2_sweep(struct worker *wrk, struct h2_sess *h2)
 				h2_del_req(wrk, r2);
 			break;
 		case H2_S_CLOS_REM:
+			if (!r2->scheduled) {
+				h2_tx_rst(wrk, h2, r2->stream,
+				    H2SE_REFUSED_STREAM);
+				h2_del_req(wrk, r2);
+				continue;
+			}
+			/* FALLTHROUGH */
 		case H2_S_CLOS_LOC:
 		case H2_S_OPEN:
 			if (h2_stream_tmo(h2, r2)) {
@@ -1088,3 +1096,21 @@ h2_rxframe(struct worker *wrk, struct h2_sess *h2)
 	}
 	return (h2e ? 0 : 1);
 }
+
+void
+h2_cleanup_waiting(struct worker *wrk, struct h2_req *r2)
+{
+	CHECK_OBJ_NOTNULL(r2, H2_REQ_MAGIC);
+	CHECK_OBJ_NOTNULL(r2->req, REQ_MAGIC);
+	CHECK_OBJ_NOTNULL(r2->h2sess, H2_SESS_MAGIC);
+
+	AN(r2->req->ws->r);
+	WS_Release(r2->req->ws, 0);
+	AN(r2->req->hash_objhead);
+	(void)HSH_DerefObjHead(wrk, &r2->req->hash_objhead);
+	AZ(r2->req->hash_objhead);
+	assert(r2->state == H2_S_CLOS_REM);
+	AN(r2->scheduled);
+	r2->scheduled = 0;
+	r2->h2sess->do_sweep = 1;
+}
diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c
index 29ba010..50cdd9a 100644
--- a/bin/varnishd/http2/cache_http2_session.c
+++ b/bin/varnishd/http2/cache_http2_session.c
@@ -431,6 +431,7 @@ static void v_matchproto_(vtr_reembark_f)
 h2_reembark(struct worker *wrk, struct req *req)
 {
 	struct sess *sp;
+	struct h2_req *r2;
 
 	sp = req->sp;
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
@@ -443,12 +444,9 @@ h2_reembark(struct worker *wrk, struct req *req)
 	/* Couldn't schedule, ditch */
 	wrk->stats->busy_wakeup--;
 	wrk->stats->busy_killed++;
-	AN (req->vcl);
-	VCL_Rel(&req->vcl);
-	Req_AcctLogCharge(wrk->stats, req);
-	Req_Release(req);
-	DSL(DBG_WAITINGLIST, req->vsl->wid, "kill from waiting list");
-	usleep(10000);
+	CAST_OBJ_NOTNULL(r2, req->transport_priv, H2_REQ_MAGIC);
+	VSLb(req->vsl, SLT_Error, "Fail to reschedule req from waiting list");
+	h2_cleanup_waiting(wrk, r2);
 }
 
 
diff --git a/bin/varnishtest/tests/r02563.vtc b/bin/varnishtest/tests/r02563.vtc
new file mode 100644
index 0000000..bfe3514
--- /dev/null
+++ b/bin/varnishtest/tests/r02563.vtc
@@ -0,0 +1,64 @@
+varnishtest "#2563: Panic after reembark failure"
+
+barrier b1 cond 2
+barrier b2 cond 2
+
+server s1 {
+	rxreq
+	expect req.url == "/foo"
+	expect req.http.client == "c1"
+	send "HTTP/1.0 200 OK\r\nConnection: close\r\n\r\n"
+	delay .2
+	barrier b1 sync
+	delay .2
+	send "line1\n"
+	delay .2
+	barrier b2 sync
+	send "line2\n"
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_backend_response {
+		set beresp.do_stream = false;
+	}
+} -start
+
+varnish v1 -cliok "param.set feature +http2"
+varnish v1 -cliok "param.set debug +failresched"
+varnish v1 -cliok "param.set debug +waitinglist"
+varnish v1 -cliok "param.set debug +syncvsl"
+
+client c1 {
+	txreq -url "/foo" -hdr "client: c1"
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 12
+	expect resp.http.x-varnish == "1001"
+} -start
+
+barrier b1 sync
+
+client c2 {
+	stream 1 {
+		txreq -url "/foo"
+		delay .2
+		barrier b2 sync
+		rxrst
+		expect rst.err == REFUSED_STREAM
+	} -start
+
+	stream 3 {
+		delay 1
+		txreq -url "/foo"
+		rxresp
+	} -run
+
+	stream 1 -wait
+} -run
+
+client c1 -wait
+
+varnish v1 -vsl_catchup
+varnish v1 -expect busy_sleep >= 1
+varnish v1 -expect busy_wakeup == 0
+varnish v1 -expect busy_killed == 1