BUG/MEDIUM: mux_quic: do not free QCS if STOP_SENDING to sent

When stream detach callback is called, the default behavior is to free
the associated QCS instance. However, QCS may be preserved in so-called
detached state if there is remaining data to sent.

This condition is checked via qcs_is_close_local() which ensures that
either FIN or a RESET_STREAM was emitted. However, this does not take
into account a scheduled STOP_SENDING emission, which can happen in case
of request abort for example.

Adjusts qcm_strm_detach() to also take into account STOP_SENDING
emission before freeing or keeping a detached QCS instance. As a
complement, QCS have to be purged after STOP_SENDING emission when
reaching completion.

On frontend side, this bug is probably only visible in case of HTTP/3
POST. When dealing with GET, FIN is most of the time received earlier,
which render STOP_SENDING unnecessary. This issue however has a bigger
impact on the backend side. In case of stream abort, for example on
timeout, the server may be left unnotified and will continue to emit
STREAM data despite QCS closure on haproxy client side.

Note that this fix also has a side effect on backend connection reuse.
Indeed it may increase the rate of QCS in detached state. This may
prevent an idle connection to be reinserted in the server pool, without
any possibility to reinsert it later. In the end this causes a lower
reuse rate. This is an issue which must be addressed in a dedicated
patch. For now, add a COUNT_IF_HOT() to report when such situation
occurs.

This should be backported to all stable releases, after a period of
observation. COUNT_IF_HOT() is unnecessary on 3.2 and below.

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 3b8f40cf671022a3e4c2c053149f78a17fef2656..550a02a8c11795ce4e99d1901fa4d22e5b13d212 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -489,7 +489,8 @@ int qcs_is_completed(struct qcs *qcs)
         * detached and everything already sent.
         */
        return (qcs->st == QC_SS_CLO && !qcs_sc(qcs)) ||
-              (qcs_is_close_local(qcs) && (qcs->flags & QC_SF_DETACH));
+              (qcs_is_close_local(qcs) && (qcs->flags & QC_SF_DETACH) &&
+               !(qcs->flags & QC_SF_TO_STOP_SENDING));
 }
 
 /* Close the remote channel of <qcs> instance. */
@@ -3004,6 +3005,12 @@ static int qcc_emit_rs_ss(struct qcc *qcc)
                        if (!(qcs->flags & (QC_SF_FIN_STREAM|QC_SF_TO_RESET)) &&
                            ((conn_is_quic(qcc->conn) && !qcs->tx.stream) || !qcs_prep_bytes(qcs))) {
                                LIST_DEL_INIT(&qcs->el_send);
+
+                               if (qcs_is_completed(qcs)) {
+                                       TRACE_STATE("add stream in purg_list", QMUX_EV_QCC_SEND|QMUX_EV_QCS_SEND, qcc->conn, qcs);
+                                       LIST_DEL_INIT(&qcs->el_send);
+                                       LIST_APPEND(&qcc->purg_list, &qcs->el_send);
+                               }
                                continue;
                        }
                }
@@ -4239,15 +4246,16 @@ static void qcm_strm_detach(struct sedesc *sd)
 
        qcc_rm_sc(qcc);
 
-       if (!qcs_is_close_local(qcs) &&
+       /* Prevent QCS free if there is remaining data to send. */
+       if ((!qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_STOP_SENDING)) &&
            !(qcc->flags & (QC_CF_ERR_CONN|QC_CF_ERRL))) {
                TRACE_STATE("remaining data, detaching qcs", QMUX_EV_STRM_END, conn, qcs);
                qcs->flags |= QC_SF_DETACH;
                qcc_refresh_timeout(qcc);
 
-               /* TODO on backend side if a QCS is detached, the connection may
-                * not be reinserted in the correct server pool (idle or avail).
-                */
+               /* TODO should detached QCS be ignored to consider conn as idle ? */
+               COUNT_IF_HOT(conn_is_back(conn),
+                            "QCS in detached state on BE side, may lower reuse rate");
 
                TRACE_LEAVE(QMUX_EV_STRM_END, qcc->conn, qcs);
                return;