From 0b222476062d76a1660892a81e9a97962dce1582 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 21 Oct 2021 22:24:31 +0200 Subject: [PATCH] MINOR: mux-h2: perform a full cycle shutdown+drain on close While in H1 we can usually close quickly, in H2 a client might be sending window updates or anything while we're sending a GOAWAY and the pending data in the socket buffers at the moment the close() is performed on the socket results in the output data being lost and an RST being emitted. One example where this happens easily is with h2spec, which randomly reports connection resets when waiting for a GOAWAY while haproxy sends it, as seen in issue #1422. With h2spec it's not window updates that are causing this but the fact that h2spec has to upload the payload that comes with invalid frames to accommodate various implementations, and does that in two different segments. When haproxy aborts on the invalid frame header, the payload was not yet received and causes an RST to be sent. Here we're dealing with this two ways: - we perform a shutdown(WR) on the connection to forcefully push pending data on a front connection after the xprt is shut and closed ; - we drain pending data - then we close This totally solves the issue with h2spec, and the extra cost is very low, especially if we consider that H2 connections are not set up and torn down often. This issue was never observed with regular clients, most likely because this pattern does not happen in regular traffic. After more testing it could make sense to backport this, at least to avoid reporting errors on h2spec tests. --- src/mux_h2.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/mux_h2.c b/src/mux_h2.c index aeaa86cae..8a7b2353d 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -1123,7 +1123,19 @@ static void h2_release(struct h2c *h2c) TRACE_DEVEL("freeing conn", H2_EV_H2C_END, conn); conn_stop_tracking(conn); - conn_full_close(conn); + + /* there might be a GOAWAY frame still pending in the TCP + * stack, and if the peer continues to send (i.e. window + * updates etc), this can result in losing the GOAWAY. For + * this reason we try to drain anything received in between. + */ + conn->flags |= CO_FL_WANT_DRAIN; + + conn_xprt_shutw(conn); + conn_xprt_close(conn); + conn_sock_shutw(conn, !conn_is_back(conn)); + conn_ctrl_close(conn); + if (conn->destroy_cb) conn->destroy_cb(conn); conn_free(conn);