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.
This commit is contained in:
Amaury Denoyelle 2023-10-26 18:17:29 +02:00
parent 9496e7e888
commit 47ed1181f2

View File

@ -219,7 +219,7 @@ static inline int qcc_is_dead(const struct qcc *qcc)
/* Connection considered dead if either : /* Connection considered dead if either :
* - remote error detected at tranport level * - remote error detected at tranport level
* - error detected locally * - error detected locally
* - MUX timeout expired or unset * - MUX timeout expired
*/ */
if (qcc->flags & (QC_CF_ERR_CONN|QC_CF_ERRL_DONE) || if (qcc->flags & (QC_CF_ERR_CONN|QC_CF_ERRL_DONE) ||
!qcc->task) { !qcc->task) {
@ -2480,6 +2480,7 @@ static struct task *qcc_timeout_task(struct task *t, void *ctx, unsigned int sta
goto out; goto out;
} }
/* Mark timeout as triggered by setting task to NULL. */
qcc->task = NULL; qcc->task = NULL;
/* TODO depending on the timeout condition, different shutdown mode /* 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; qcc->proxy = prx;
/* haproxy timeouts */ /* haproxy timeouts */
qcc->task = NULL;
if (conn_is_back(qcc->conn)) { if (conn_is_back(qcc->conn)) {
qcc->timeout = prx->timeout.server; qcc->timeout = prx->timeout.server;
qcc->shut_timeout = tick_isset(prx->timeout.serverfin) ? qcc->shut_timeout = tick_isset(prx->timeout.serverfin) ?
@ -2585,7 +2585,9 @@ static int qmux_init(struct connection *conn, struct proxy *prx,
prx->timeout.clientfin : prx->timeout.client; prx->timeout.clientfin : prx->timeout.client;
} }
if (tick_isset(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(); qcc->task = task_new_here();
if (!qcc->task) { if (!qcc->task) {
TRACE_ERROR("timeout task alloc failure", QMUX_EV_QCC_NEW); TRACE_ERROR("timeout task alloc failure", QMUX_EV_QCC_NEW);
@ -2593,8 +2595,8 @@ static int qmux_init(struct connection *conn, struct proxy *prx,
} }
qcc->task->process = qcc_timeout_task; qcc->task->process = qcc_timeout_task;
qcc->task->context = qcc; qcc->task->context = qcc;
qcc->task->expire = tick_add(now_ms, qcc->timeout); qcc->task->expire = tick_add_ifset(now_ms, qcc->timeout);
}
qcc_reset_idle_start(qcc); qcc_reset_idle_start(qcc);
LIST_INIT(&qcc->opening_list); 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); TRACE_STATE("killing dead connection", QMUX_EV_STRM_END, qcc->conn);
goto release; goto release;
} }
else if (qcc->task) { else {
TRACE_DEVEL("refreshing connection's timeout", QMUX_EV_STRM_END, qcc->conn); TRACE_DEVEL("refreshing connection's timeout", QMUX_EV_STRM_END, qcc->conn);
qcc_refresh_timeout(qcc); qcc_refresh_timeout(qcc);
} }
else {
TRACE_DEVEL("completed", QMUX_EV_STRM_END, qcc->conn);
}
TRACE_LEAVE(QMUX_EV_STRM_END, qcc->conn); TRACE_LEAVE(QMUX_EV_STRM_END, qcc->conn);
return; return;