From: Willy Tarreau <w@1wt.eu>
Date: Fri, 7 Jun 2019 06:20:46 +0000 (+0200)
Subject: BUG/MEDIUM: mux-h2: make sure the connection timeout is always set
X-Git-Tag: v2.0-dev7~34
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7348119fb22bf761c33e06e8a092bd006660cc81;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: mux-h2: make sure the connection timeout is always set

There seems to be a tricky case in the H2 mux related to stream flow
control versus buffer a full situation : is a large response cannot
be entirely sent to the client due to the stream window being too
small, the stream is paused with the SFCTL flag. Then the upper
layer stream might get bored and expire this stream. It will then
shut it down first. But the shutdown operation might fail if the
mux buffer is full, resulting in the h2s being subscribed to the
deferred_shut event with the stream *not* added to the send_list
since it's blocked in SFCTL. In the mean time the upper layer completely
closes, calling h2_detach(). There we have a send_wait (the pending
shutw), the stream is marked with SFCTL so we orphan it.

Then if the client finally reads all the data that were clogging the
buffer, the send_list is run again, but our stream is not there. From
this point, the connection's stream list is not empty, the mux buffer
is empty, so the connection's timeout is not set. If the client
disappears without updating the stream's window, nothing will expire
the connection.

This patch makes sure we always keep the connection timeout updated.
There might be finer solutions, such as checking that there are still
living streams in the connection (i.e. streams not blocked in SFCTL
state), though this is not necessarily trivial nor useful, since the
client timeout is the same for the upper level stream and the connection
anyway.

This patch needs to be backported to 1.9 and 1.8 after some observation.
---

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 65cefa4bd2..39770e83d0 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2989,12 +2989,8 @@ static int h2_process(struct h2c *h2c)
 		h2_release_mbuf(h2c);
 
 	if (h2c->task) {
-		if (eb_is_empty(&h2c->streams_by_id) || br_data(h2c->mbuf)) {
-			h2c->task->expire = tick_add(now_ms, h2c->last_sid < 0 ? h2c->timeout : h2c->shut_timeout);
-			task_queue(h2c->task);
-		}
-		else
-			h2c->task->expire = TICK_ETERNITY;
+		h2c->task->expire = tick_add(now_ms, h2c->last_sid < 0 ? h2c->timeout : h2c->shut_timeout);
+		task_queue(h2c->task);
 	}
 
 	h2_send(h2c);
@@ -3244,12 +3240,8 @@ static void h2_detach(struct conn_stream *cs)
 		h2_release(h2c);
 	}
 	else if (h2c->task) {
-		if (eb_is_empty(&h2c->streams_by_id) || br_data(h2c->mbuf)) {
-			h2c->task->expire = tick_add(now_ms, h2c->last_sid < 0 ? h2c->timeout : h2c->shut_timeout);
-			task_queue(h2c->task);
-		}
-		else
-			h2c->task->expire = TICK_ETERNITY;
+		h2c->task->expire = tick_add(now_ms, h2c->last_sid < 0 ? h2c->timeout : h2c->shut_timeout);
+		task_queue(h2c->task);
 	}
 }