BUG/MINOR: mux-quic: fix early close if unset client timeout

When no client timeout is defined in the configuration, QCC timeout task
is never allocated. However, a NULL timeout task is also used as a
criteria in qcc_is_dead() to consider that the MUX instance should be
released as timeout stroke earlier.

This bug causes every connection to be closed by haproxy side with a
CONNECTION_CLOSE. This is notable when using several streams per
connection with only the first stream completed and the others failed.

To fix this, change timeout task allocation policy. It is now always
allocated. This means that if no timeout is defined, it will never be
run. This is not considered a waste of resource as no timeout in the
configuration is considered as an exception case. However, this has the
advantage to simplify the rest of the code which can now check for the
task instance without having an extra check on the timeout value.

This bug is labelled as minor as it only occurs if no timeout client is
defined which reports warning on startup as it may caused unexpected
behavior.

This bug should be backported up to 2.6.

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 429b22f9cbcdc4df27d68eea0dd0529d26a8cc40..8d0127f3737aa9cc835404c692c49cfb0fcc399c 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -219,7 +219,7 @@ static inline int qcc_is_dead(const struct qcc *qcc)
        /* Connection considered dead if either :
         * - remote error detected at tranport level
         * - error detected locally
-        * - MUX timeout expired or unset
+        * - MUX timeout expired
         */
        if (qcc->flags & (QC_CF_ERR_CONN|QC_CF_ERRL_DONE) ||
            !qcc->task) {
@@ -2480,6 +2480,7 @@ static struct task *qcc_timeout_task(struct task *t, void *ctx, unsigned int sta
                goto out;
        }
 
+       /* Mark timeout as triggered by setting task to NULL. */
        qcc->task = NULL;
 
        /* TODO depending on the timeout condition, different shutdown mode
@@ -2573,7 +2574,6 @@ static int qmux_init(struct connection *conn, struct proxy *prx,
 
        qcc->proxy = prx;
        /* haproxy timeouts */
-       qcc->task = NULL;
        if (conn_is_back(qcc->conn)) {
                qcc->timeout = prx->timeout.server;
                qcc->shut_timeout = tick_isset(prx->timeout.serverfin) ?
@@ -2585,16 +2585,18 @@ static int qmux_init(struct connection *conn, struct proxy *prx,
                                    prx->timeout.clientfin : prx->timeout.client;
        }
 
-       if (tick_isset(qcc->timeout)) {
-               qcc->task = task_new_here();
-               if (!qcc->task) {
-                       TRACE_ERROR("timeout task alloc failure", QMUX_EV_QCC_NEW);
-                       goto fail_no_timeout_task;
-               }
-               qcc->task->process = qcc_timeout_task;
-               qcc->task->context = qcc;
-               qcc->task->expire = tick_add(now_ms, qcc->timeout);
+       /* Always allocate task even if timeout is unset. In MUX code, if task
+        * is NULL, it indicates that a timeout has stroke earlier.
+        */
+       qcc->task = task_new_here();
+       if (!qcc->task) {
+               TRACE_ERROR("timeout task alloc failure", QMUX_EV_QCC_NEW);
+               goto fail_no_timeout_task;
        }
+       qcc->task->process = qcc_timeout_task;
+       qcc->task->context = qcc;
+       qcc->task->expire = tick_add_ifset(now_ms, qcc->timeout);
+
        qcc_reset_idle_start(qcc);
        LIST_INIT(&qcc->opening_list);
 
@@ -2676,13 +2678,10 @@ static void qmux_strm_detach(struct sedesc *sd)
                TRACE_STATE("killing dead connection", QMUX_EV_STRM_END, qcc->conn);
                goto release;
        }
-       else if (qcc->task) {
+       else {
                TRACE_DEVEL("refreshing connection's timeout", QMUX_EV_STRM_END, qcc->conn);
                qcc_refresh_timeout(qcc);
        }
-       else {
-               TRACE_DEVEL("completed", QMUX_EV_STRM_END, qcc->conn);
-       }
 
        TRACE_LEAVE(QMUX_EV_STRM_END, qcc->conn);
        return;