[master] 70d49d5 h2: Fix reembark failure handling
Dag Haavi Finstad
daghf at varnish-software.com
Wed Mar 14 10:29:10 UTC 2018
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
More information about the varnish-commit
mailing list