BUG/MINOR: mux-h2: refresh the idle_timer when the mux is empty

There's a rare case where on long fat pipes, we can see the keep-alive
timeout trigger before the end of the transfer of the last large object,
and the connection closed a bit quickly after the end of the transfer
because a GOAWAY is queued. The data are not destroyed, except that
the WINDOW_UPDATES from the client arriving late while the last data
are being drained by the socket buffers may at some point trigger a
reset, and some clients might choke a bit too early on these. Let's
make sure we only arm the idle_start timestamp once the output buffer
is empty. Of course it will still not cover for the data pending in the
socket buffers but it will at least let those in the buffer leave in
peace. More elaborate options can be used to protect the data in the
kernel buffers, such as the one described in GH issue #5.

It's very likely that this old issue was emphasized by the following
commit in 2.6:
  15a4733d5 ("BUG/MEDIUM: mux-h2: make use of http-request and keep-alive timeouts")

and the behavior probably changed again with this one in 2.8, which
was backported to 2.7 and scheduled for 2.6:
  d38d8c6cc ("BUG/MEDIUM: mux-h2: make sure control frames do not refresh the idle timeout")

As such this patch should be backported to 2.6 after some observation
period.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index b4dbb88ffb8df14b2cfb35282b5c707d4de64c20..2e21088ae1d2f517ea113f23d8b93adf6cbf6a61 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -75,7 +75,7 @@ struct h2c {
 
        int timeout;        /* idle timeout duration in ticks */
        int shut_timeout;   /* idle timeout duration in ticks after GOAWAY was sent */
-       int idle_start;     /* date of the last time the connection went idle */
+       int idle_start;     /* date of the last time the connection went idle (no stream + empty mbuf), or the start of current http req */
        /* 32-bit hole here */
        unsigned int nb_streams;  /* number of streams in the tree */
        unsigned int nb_sc;       /* number of attached stream connectors */
@@ -3411,7 +3411,8 @@ static void h2_process_demux(struct h2c *h2c)
                        }
 
                        /* transition to HEADERS frame ends the keep-alive idle
-                        * timer and starts the http-request idle delay.
+                        * timer and starts the http-request idle delay. It uses
+                        * the idle_start timer as well.
                         */
                        if (hdr.ft == H2_FT_HEADERS)
                                h2c->idle_start = now_ms;
@@ -3823,6 +3824,7 @@ static int h2_send(struct h2c *h2c)
                if (h2c->flags & H2_CF_RCVD_SHUT)
                        h2c->flags |= H2_CF_ERROR;
                b_reset(br_tail(h2c->mbuf));
+               h2c->idle_start = now_ms;
                return 1;
        }
 
@@ -3931,6 +3933,8 @@ static int h2_send(struct h2c *h2c)
        /* We're done, no more to send */
        if (!(conn->flags & CO_FL_WAIT_XPRT) && !br_data(h2c->mbuf)) {
                TRACE_DEVEL("leaving with everything sent", H2_EV_H2C_SEND, h2c->conn);
+               if (!h2c->nb_sc)
+                       h2c->idle_start = now_ms;
                goto end;
        }
 
@@ -4366,7 +4370,7 @@ static void h2_detach(struct sedesc *sd)
        sess = h2s->sess;
        h2c = h2s->h2c;
        h2c->nb_sc--;
-       if (!h2c->nb_sc)
+       if (!h2c->nb_sc && !br_data(h2c->mbuf))
                h2c->idle_start = now_ms;
 
        if ((h2c->flags & (H2_CF_IS_BACK|H2_CF_DEM_TOOMANY)) == H2_CF_DEM_TOOMANY &&