[master] 3a9681dec Revert "More work on refactoring req-cleanup"

[Dridi Boukelmoune]

commit 3a9681dec62144e31822a6b9aa776260d7e67677
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Oct 15 14:26:33 2019 +0200

    Revert "More work on refactoring req-cleanup"
    
    This reverts commit 6233b08842d8ea48e78b9fc5381a26a2c753862e.
    
    Conflicts:
            bin/varnishd/cache/cache_req.c
    
    The conflict was introduced with the handling of h2 stream 0 pseudo
    request: 51127b4600f095fb086c5115905b90ea91731d4f. This fixes a
    regression where timeout_idle could be circumvented by a periodic
    CRLF, except that it has no real consequence like holding onto a
    worker thread thanks to timeout_linger kicking in as expected.
    
    In addition to reverting the change, test coverage is added.

diff --git a/bin/varnishd/cache/cache_req.c b/bin/varnishd/cache/cache_req.c
index def146b04..b13440e47 100644
--- a/bin/varnishd/cache/cache_req.c
+++ b/bin/varnishd/cache/cache_req.c
@@ -43,8 +43,8 @@
 
 #include "vtim.h"
 
-static void
-req_AcctLogCharge(struct VSC_main_wrk *ds, struct req *req)
+void
+Req_AcctLogCharge(struct VSC_main_wrk *ds, struct req *req)
 {
 	struct acct_req *a;
 
@@ -165,7 +165,8 @@ Req_Release(struct req *req)
 #include "tbl/acct_fields_req.h"
 
 	AZ(req->vcl);
-	AZ(req->vsl->wid);
+	if (req->vsl->wid)
+		VSL_End(req->vsl);
 	sp = req->sp;
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	pp = sp->pool;
@@ -219,8 +220,8 @@ Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req)
 		VCL_Recache(wrk, &req->vcl);
 
 	/* Charge and log byte counters */
-	req_AcctLogCharge(wrk->stats, req);
 	if (req->vsl->wid) {
+		Req_AcctLogCharge(wrk->stats, req);
 		if (req->vsl->wid != sp->vxid)
 			VSL_End(req->vsl);
 		else
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index d32c8dde8..0fb461c61 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -350,6 +350,7 @@ void Req_Release(struct req *);
 void Req_Rollback(struct req *req);
 void Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req);
 void Req_Fail(struct req *req, enum sess_close reason);
+void Req_AcctLogCharge(struct VSC_main_wrk *, struct req *);
 
 /* cache_req_body.c */
 int VRB_Ignore(struct req *);
diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c
index c289bbbf1..fcd914ba4 100644
--- a/bin/varnishd/http1/cache_http1_fsm.c
+++ b/bin/varnishd/http1/cache_http1_fsm.c
@@ -338,7 +338,7 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 			if (hs < HTC_S_EMPTY) {
 				req->acct.req_hdrbytes +=
 				    req->htc->rxbuf_e - req->htc->rxbuf_b;
-				Req_Cleanup(sp, wrk, req);
+				Req_AcctLogCharge(wrk->stats, req);
 				Req_Release(req);
 				switch (hs) {
 				case HTC_S_CLOSE:
@@ -359,7 +359,6 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 			}
 			if (hs == HTC_S_IDLE) {
 				wrk->stats->sess_herd++;
-				Req_Cleanup(sp, wrk, req);
 				Req_Release(req);
 				SES_Wait(sp, &HTTP1_transport);
 				return;
diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c
index 4fd379cd9..8eb5c0506 100644
--- a/bin/varnishd/http2/cache_http2_session.c
+++ b/bin/varnishd/http2/cache_http2_session.c
@@ -240,7 +240,7 @@ h2_ou_rel(struct worker *wrk, struct req *req)
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	AZ(req->vcl);
-	Req_Cleanup(req->sp, wrk, req);
+	Req_AcctLogCharge(wrk->stats, req);
 	Req_Release(req);
 	return (0);
 }
diff --git a/bin/varnishtest/tests/b00067.vtc b/bin/varnishtest/tests/b00067.vtc
index 6eb38996c..612f5fcd5 100644
--- a/bin/varnishtest/tests/b00067.vtc
+++ b/bin/varnishtest/tests/b00067.vtc
@@ -37,6 +37,28 @@ client c2 {
 	expect_close
 } -start
 
+client c3 {
+	loop 3 {
+		# send a periodic CRLF
+		delay 0.3
+		sendhex 0d0a
+	}
+	delay 0.3
+	expect_close
+} -start
+
+client c4 {
+	txreq
+	rxresp
+	loop 3 {
+		# send a periodic CRLF
+		delay 0.3
+		sendhex 0d0a
+	}
+	delay 0.3
+	expect_close
+} -start
+
 client c1u -connect "${tmpdir}/v1.sock" {
 	txreq
 	rxresp
@@ -57,7 +79,33 @@ client c2u -connect "${tmpdir}/v1.sock" {
 	expect_close
 } -start
 
+client c3u -connect "${tmpdir}/v1.sock" {
+	loop 3 {
+		# send a periodic CRLF
+		delay 0.3
+		sendhex 0d0a
+	}
+	delay 0.3
+	expect_close
+} -start
+
+client c4u -connect "${tmpdir}/v1.sock" {
+	txreq
+	rxresp
+	loop 3 {
+		# send a periodic CRLF
+		delay 0.3
+		sendhex 0d0a
+	}
+	delay 0.3
+	expect_close
+} -start
+
 client c1 -wait
 client c2 -wait
+client c3 -wait
+client c4 -wait
 client c1u -wait
 client c2u -wait
+client c3u -wait
+client c4u -wait