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.
(cherry picked from commit 47ed1181f29a56ccb04e5b80ef4f9414466ada7a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/mux_quic.c b/src/mux_quic.c
index c44d536..85eae4d 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -219,7 +219,7 @@
/* 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 @@
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 @@
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 @@
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 @@
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;