[master] 3e015a1 Do all of the vfc setup and init in one place and only when needed

Nils Goroll nils.goroll at uplex.de
Wed Mar 22 15:07:05 CET 2017


commit 3e015a1a8e256c65d4536390b6db098740ca9a5e
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Wed Mar 22 14:17:55 2017 +0100

    Do all of the vfc setup and init in one place and only when needed
    
    Previously, we would be left with a set up but uninitialized req->vfc
    when reembarking a request on the waitinglist because the set up was
    done in CNT_Request, but transport->req_body was only called in cnt_recv.
    
    Fixes #2266

diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 524a21e..2086966 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -90,6 +90,14 @@ cnt_transport(struct worker *wrk, struct req *req)
 		return (REQ_FSM_DONE);
 	}
 
+	if (req->req_body_status < REQ_BODY_TAKEN) {
+		AN(req->transport->req_body != NULL);
+		VFP_Setup(req->vfc);
+		req->vfc->http = req->http;
+		req->vfc->wrk = wrk;
+		req->transport->req_body(req);
+	}
+
 	HTTP_Copy(req->http0, req->http);	// For ESI & restart
 	req->req_step = R_STP_RECV;
 	return (REQ_FSM_MORE);
@@ -791,13 +799,9 @@ cnt_recv(struct worker *wrk, struct req *req)
 
 	http_CollectHdr(req->http, H_Cache_Control);
 
-	if (req->transport->req_body != NULL) {
-		req->transport->req_body(req);
-
-		if (req->req_body_status == REQ_BODY_FAIL) {
-			req->doclose = SC_OVERLOAD;
-			return (REQ_FSM_DONE);
-		}
+	if (req->req_body_status == REQ_BODY_FAIL) {
+		req->doclose = SC_OVERLOAD;
+		return (REQ_FSM_DONE);
 	}
 
 	VCL_recv_method(req->vcl, wrk, req, NULL, NULL);
@@ -967,13 +971,8 @@ CNT_Request(struct worker *wrk, struct req *req)
 
 	AN(req->vsl->wid & VSL_CLIENTMARKER);
 
-	req->wrk = wrk;
+	req->vfc->wrk = req->wrk = wrk;
 	wrk->vsl = req->vsl;
-
-	VFP_Setup(req->vfc);
-	req->vfc->http = req->http;
-	req->vfc->wrk = wrk;
-
 	for (nxt = REQ_FSM_MORE; nxt == REQ_FSM_MORE; ) {
 		/*
 		 * This is a good place to be paranoid about the various
diff --git a/bin/varnishtest/tests/r02266.vtc b/bin/varnishtest/tests/r02266.vtc
new file mode 100644
index 0000000..5fed4d6
--- /dev/null
+++ b/bin/varnishtest/tests/r02266.vtc
@@ -0,0 +1,50 @@
+varnishtest "request with body parked on waitinglist - almost identical to c00013.vtc"
+
+barrier b1 cond 2
+barrier b2 cond 2
+
+server s1 {
+	rxreq
+	expect req.url == "/foo"
+	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 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 {
+	txreq -url "/foo" -hdr "client: c2" -body 1
+	delay .2
+	barrier b2 sync
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 12
+	expect resp.http.x-varnish == "1004 1002"
+} -run
+
+client c1 -wait
+
+varnish v1 -expect busy_sleep >= 1
+varnish v1 -expect busy_wakeup >= 1
diff --git a/include/tbl/req_body.h b/include/tbl/req_body.h
index 9878e9c..5b4716a 100644
--- a/include/tbl/req_body.h
+++ b/include/tbl/req_body.h
@@ -32,6 +32,7 @@
 REQ_BODY(INIT)
 REQ_BODY(WITHOUT_LEN)
 REQ_BODY(WITH_LEN)
+/* states >= TAKEN imply that no body is to be read */
 REQ_BODY(TAKEN)
 REQ_BODY(CACHED)
 REQ_BODY(FAIL)



More information about the varnish-commit mailing list