[6.0] 62db24848 respect `send_timeout` for "dripping" http1 writes

Reza Naghibi reza at naghibi.com
Tue Jun 16 16:00:09 UTC 2020


commit 62db248488e450c8dc9b276ab1b9420201deb958
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon Jan 27 14:18:28 2020 +0100

    respect `send_timeout` for "dripping" http1 writes
    
    Previously, we only checked `v1l->deadline` (which gets initialized
    from the `send_timeout` session attribute or parameter) only for short
    writes, so for successful "dripping" http1 writes (streaming from a
    backend busy object with small chunks), we did not respect the
    timeout.
    
    This patch restructures `V1L_Flush()` to always check the deadline
    before a write by turning a `while() { ... }` into a `do { ... }
    while` with the same continuation criteria: `while (i != v1l->liov)`
    is turned into `if (i == v1l->liov) break;` and `while (i > 0 || errno
    == EWOULDBLOCK)` is kept to retry short writes.
    
    This also reduces the `writev()` call sites to one.
    
    Fixes #3189
    
     Conflicts:
            bin/varnishd/http1/cache_http1_line.c
            bin/varnishtest/tests/s00011.vtc

diff --git a/bin/varnishd/http1/cache_http1_line.c b/bin/varnishd/http1/cache_http1_line.c
index e7be7c8c2..be0214afb 100644
--- a/bin/varnishd/http1/cache_http1_line.c
+++ b/bin/varnishd/http1/cache_http1_line.c
@@ -201,18 +201,8 @@ V1L_Flush(const struct worker *wrk)
 			v1l->iov[v1l->ciov].iov_len = 0;
 		}
 
-		i = writev(*v1l->wfd, v1l->iov, v1l->niov);
-		if (i > 0)
-			v1l->cnt += i;
-		while (i != v1l->liov && (i > 0 || errno == EWOULDBLOCK)) {
-			/* Remove sent data from start of I/O vector,
-			 * then retry; we hit a timeout, but some data
-			 * was sent.
-			 *
-			 * XXX: Add a "minimum sent data per timeout
-			 * counter to prevent slowlaris attacks
-			*/
-
+		i = 0;
+		do {
 			if (VTIM_real() - v1l->t0 > cache_param->send_timeout) {
 				VSLb(v1l->vsl, SLT_Debug,
 				    "Hit total send timeout, "
@@ -222,16 +212,28 @@ V1L_Flush(const struct worker *wrk)
 				break;
 			}
 
+			i = writev(*v1l->wfd, v1l->iov, v1l->niov);
+			if (i > 0)
+				v1l->cnt += i;
+
+			if (i == v1l->liov)
+				break;
+
+			/* we hit a timeout, and some data may have been sent:
+			 * Remove sent data from start of I/O vector, then retry
+			 *
+			 * XXX: Add a "minimum sent data per timeout counter to
+			 * prevent slowloris attacks
+			 */
+
 			VSLb(v1l->vsl, SLT_Debug,
 			    "Hit idle send timeout, wrote = %zd/%zd; retrying",
 			    i, v1l->liov);
 
 			if (i > 0)
 				v1l_prune(v1l, i);
-			i = writev(*v1l->wfd, v1l->iov, v1l->niov);
-			if (i > 0)
-				v1l->cnt += i;
-		}
+		} while (i > 0 || errno == EWOULDBLOCK);
+
 		if (i <= 0) {
 			v1l->werr++;
 			VSLb(v1l->vsl, SLT_Debug,
diff --git a/bin/varnishtest/tests/r03189.vtc b/bin/varnishtest/tests/r03189.vtc
new file mode 100644
index 000000000..cf3d466c9
--- /dev/null
+++ b/bin/varnishtest/tests/r03189.vtc
@@ -0,0 +1,28 @@
+varnishtest "h1 send_timeout and streaming of dripping chunks"
+
+barrier b cond 2
+
+server s1 {
+	rxreq
+	txresp -nolen -hdr "Transfer-Encoding: chunked"
+	chunkedlen 1
+	delay 1
+	non_fatal
+	barrier b sync
+	chunkedlen 1
+	delay 1
+	chunkedlen 0
+} -start
+
+varnish v1				\
+	-arg "-p idle_send_timeout=.1"	\
+	-arg "-p send_timeout=.8"	\
+	-vcl+backend { } -start
+
+client c1 {
+	txreq
+	rxresphdrs
+	rxchunk
+	barrier b sync
+	expect_close
+} -run


More information about the varnish-commit mailing list