]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: mux-h2: try to wait for the peer to read the GOAWAY
authorWilly Tarreau <w@1wt.eu>
Fri, 8 Nov 2024 09:02:47 +0000 (10:02 +0100)
committerWilly Tarreau <w@1wt.eu>
Fri, 8 Nov 2024 13:31:07 +0000 (14:31 +0100)
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.

src/mux_h2.c

index 3d0509c5d8f0323a37924aecd8ecfa9a6791fb38..b65518c0042d737603cbea2afb62ac20d2ba4108 100644 (file)
@@ -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: