BUG/MEDIUM: mux-h2: do not enforce timeout on long connections

Alexandre Derumier reported issue #308 in which the client timeout will
strike on an H2 mux when it's shorter than the server's response time.
What happens in practice is that there is no activity on the connection
and there's no data pending on output so we can expire it. But this does
not take into account the possibility that some streams are in fact
waiting for the data layer above. So what we do now is that we enforce
the timeout when:
  - there are no more streams
  - some data are pending in the output buffer
  - some streams are blocked on the connection's flow control
  - some streams are blocked on their own flow control
  - some streams are in the send/sending list

In all other cases the connection will not timeout as it means that some
streams are actively used by the data layer.

This fix must be backported to 2.0, 1.9 and probably 1.8 as well. It
depends on the new "blocked_list" field introduced by "MINOR: mux-h2:
add a per-connection list of blocked streams". It would be nice to
also backport "ebtree: make eb_is_empty() and eb_is_dup() take a const"
to avoid a build warning.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 94d387a85e36672f452358570651731a57a7546e..2c295fcb5d5be074ec835bdcbbf1737f3e059e9b 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -548,8 +548,28 @@ static void h2_trace(enum trace_level level, uint64_t mask, const struct trace_s
        }
 }
 
+/* returns true if the connection is allowed to expire, false otherwise. A
+ * connection may expire when:
+ *   - it has no stream
+ *   - it has data in the mux buffer
+ *   - it has streams in the blocked list
+ *   - it has streams in the fctl list
+ *   - it has streams in the send list
+ * Otherwise it means some streams are waiting in the data layer and it should
+ * not expire.
+ */
+static inline int h2c_may_expire(const struct h2c *h2c)
+{
+       return eb_is_empty(&h2c->streams_by_id) ||
+              br_data(h2c->mbuf) ||
+              !LIST_ISEMPTY(&h2c->blocked_list) ||
+              !LIST_ISEMPTY(&h2c->fctl_list) ||
+              !LIST_ISEMPTY(&h2c->send_list) ||
+              !LIST_ISEMPTY(&h2c->sending_list);
+}
+
 static __inline int
-h2c_is_dead(struct h2c *h2c)
+h2c_is_dead(const struct h2c *h2c)
 {
        if (eb_is_empty(&h2c->streams_by_id) &&     /* don't close if streams exist */
            ((h2c->conn->flags & CO_FL_ERROR) ||    /* errors close immediately */
@@ -3551,7 +3571,10 @@ static int h2_process(struct h2c *h2c)
                h2_release_mbuf(h2c);
 
        if (h2c->task) {
-               h2c->task->expire = tick_add(now_ms, h2c->last_sid < 0 ? h2c->timeout : h2c->shut_timeout);
+               if (h2c_may_expire(h2c))
+                       h2c->task->expire = tick_add(now_ms, h2c->last_sid < 0 ? h2c->timeout : h2c->shut_timeout);
+               else
+                       h2c->task->expire = TICK_ETERNITY;
                task_queue(h2c->task);
        }
 
@@ -3590,6 +3613,14 @@ static struct task *h2_timeout_task(struct task *t, void *context, unsigned shor
                return t;
        }
 
+       if (h2c && !h2c_may_expire(h2c)) {
+               /* we do still have streams but all of them are idle, waiting
+                * for the data layer, so we must not enforce the timeout here.
+                */
+               t->expire = TICK_ETERNITY;
+               return t;
+       }
+
        task_destroy(t);
 
        if (!h2c) {
@@ -3829,7 +3860,10 @@ static void h2_detach(struct conn_stream *cs)
                h2_release(h2c);
        }
        else if (h2c->task) {
-               h2c->task->expire = tick_add(now_ms, h2c->last_sid < 0 ? h2c->timeout : h2c->shut_timeout);
+               if (h2c_may_expire(h2c))
+                       h2c->task->expire = tick_add(now_ms, h2c->last_sid < 0 ? h2c->timeout : h2c->shut_timeout);
+               else
+                       h2c->task->expire = TICK_ETERNITY;
                task_queue(h2c->task);
                TRACE_DEVEL("leaving, refreshing connection's timeout", H2_EV_STRM_END, h2c->conn);
        }