From: Willy Tarreau <w@1wt.eu>
Date: Fri, 8 Nov 2024 09:02:47 +0000 (+0100)
Subject: BUG/MEDIUM: mux-h2: try to wait for the peer to read the GOAWAY
X-Git-Tag: v3.1-dev12~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3ed936168842e8120940cf821843b876bd70dadf;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: mux-h2: try to wait for the peer to read the GOAWAY

When timeout http-keep-alive is very short (e.g. 10ms), it's possible
sometimes for a client to face truncated responses due to an early
close that happens while the system is still pushing the last data,
colliding with the client's WINDOW_UPDATEs that trigger RSTs.

Here we're trying to do better: first we send a GOAWAY on timeout, then
we wait up to clientfin/client timeout for the peer to react so that we
don't immediately close. This is sufficient to avoid truncation as soon
as the timeout is more than a few hundred ms.

It's not certain it should be backported, because it's a bit sensistive
and might possibly fall into certain edge cases.
---

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 3d0509c5d8..b65518c004 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -847,7 +847,14 @@ static void h2c_update_timeout(struct h2c *h2c)
 			h2c->task->expire = tick_add_ifset(now_ms, h2c->timeout);
 		} else {
 			/* no stream, no output data */
-			if (!(h2c->flags & H2_CF_IS_BACK)) {
+			if (h2c->flags & (H2_CF_GOAWAY_SENT|H2_CF_GOAWAY_FAILED)) {
+				/* GOAWAY sent (or failed), closing in progress */
+				int exp = tick_add_ifset(now_ms, h2c->shut_timeout);
+
+				h2c->task->expire = tick_first(h2c->task->expire, exp);
+				is_idle_conn = 1;
+			}
+			else if (!(h2c->flags & H2_CF_IS_BACK)) {
 				int to;
 
 				if (h2c->max_id > 0 && !b_data(&h2c->dbuf) &&
@@ -865,14 +872,6 @@ static void h2c_update_timeout(struct h2c *h2c)
 				is_idle_conn = 1;
 			}
 
-			if (h2c->flags & (H2_CF_GOAWAY_SENT|H2_CF_GOAWAY_FAILED)) {
-				/* GOAWAY sent (or failed), closing in progress */
-				int exp = tick_add_ifset(now_ms, h2c->shut_timeout);
-
-				h2c->task->expire = tick_first(h2c->task->expire, exp);
-				is_idle_conn = 1;
-			}
-
 			/* if a timeout above was not set, fall back to the default one */
 			if (!tick_isset(h2c->task->expire))
 				h2c->task->expire = tick_add_ifset(now_ms, h2c->timeout);
@@ -5036,6 +5035,20 @@ struct task *h2_timeout_task(struct task *t, void *context, unsigned int state)
 			conn_delete_from_tree(h2c->conn);
 
 		HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+
+		/* Try to gracefully close idle connections by sending a GOAWAY first,
+		 * and then waiting for the fin timeout.
+		 */
+		if (!br_data(h2c->mbuf) && h2c_may_expire(h2c) &&
+		    !(h2c->flags & (H2_CF_GOAWAY_SENT|H2_CF_GOAWAY_FAILED))) {
+			h2c_error(h2c, H2_ERR_NO_ERROR);
+			if (h2_send(h2c))
+				tasklet_wakeup(h2c->wait_event.tasklet);
+			t->expire = tick_add_ifset(now_ms, h2c->shut_timeout);
+			if (!tick_isset(t->expire))
+				t->expire = tick_add_ifset(now_ms, h2c->timeout);
+			return t;
+		}
 	}
 
 do_leave: