MINOR: session: do not release conn in session_check_idle_conn()

session_check_idle_conn() is called to flag a connection already
inserted in a session list as idle. If the session limit on the number
of idle connections (max-session-srv-conns) is exceeded, the connection
is removed from the session list.

In addition to the connection removal, session_check_idle_conn()
directly calls MUX destroy callback on the connection. This means the
connection is freed by the function itself and should not be used by the
caller anymore.

This is not practical when an alternative connection closure method
should be used, such as a graceful shutdown with QUIC. As such, remove
MUX destroy invokation : this is now the responsability of the caller to
either close or release immediately the connection.

diff --git a/include/haproxy/session.h b/include/haproxy/session.h
index 8ec1f7df5596593a6c0e6be0fa37f005d824c059..6b1824aba68f42610bad92c2e1b8052f15ed82df 100644 (file)
--- a/include/haproxy/session.h
+++ b/include/haproxy/session.h
@@ -225,8 +225,12 @@ static inline int session_add_conn(struct session *sess, struct connection *conn
        return 1;
 }
 
-/* Returns 0 if the session can keep the idle conn, -1 if it was destroyed. The
- * connection must be private.
+/* Check that session <sess> is able to keep idle connection <conn>. This must
+ * be called after insertion of a private connection into session unless
+ * connection is or will be soon active.
+ *
+ * Returns 0 if the connection is kept or is not attached to the session, else
+ * non-zero if the connection was explicitely removed from session.
  */
 static inline int session_check_idle_conn(struct session *sess, struct connection *conn)
 {
@@ -243,7 +247,6 @@ static inline int session_check_idle_conn(struct session *sess, struct connectio
        if (sess->idle_conns >= sess->fe->max_out_conns) {
                session_unown_conn(sess, conn);
                conn->owner = NULL;
-               conn->mux->destroy(conn->ctx);
                return -1;
        }
        else {

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index ce607422bcc998db3e7c2205f45530b4f0a26ff4..f91abf20706cbee7361c25f1769a464841df64bc 100644 (file)
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -3738,7 +3738,7 @@ static void fcgi_detach(struct sedesc *sd)
                                 */
                                HA_ATOMIC_OR(&fconn->wait_event.tasklet->state, TASK_F_USR1);
                                if (session_check_idle_conn(fconn->conn->owner, fconn->conn) != 0) {
-                                       /* The connection is destroyed, let's leave */
+                                       fconn->conn->mux->destroy(fconn);
                                        TRACE_DEVEL("outgoing connection killed", FCGI_EV_STRM_END|FCGI_EV_FCONN_ERR);
                                        return;
                                }

diff --git a/src/mux_h1.c b/src/mux_h1.c
index 1e9c2b0decc93a25478c2b8809d2efdd09d04021..a986483ef2bc467f6c4f2e2d7d90538adcd346f7 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -1150,8 +1150,8 @@ static int h1s_finish_detach(struct h1s *h1s)
                         */
                        HA_ATOMIC_OR(&h1c->wait_event.tasklet->state, TASK_F_USR1);
                        if (session_check_idle_conn(sess, h1c->conn)) {
-                               /* The connection got destroyed, let's leave */
-                               TRACE_DEVEL("outgoing connection killed", H1_EV_STRM_END|H1_EV_H1C_END);
+                               TRACE_DEVEL("outgoing connection rejected", H1_EV_STRM_END|H1_EV_H1C_END, h1c->conn);
+                               h1c->conn->mux->destroy(h1c);
                                goto released;
                        }
                }

diff --git a/src/mux_h2.c b/src/mux_h2.c
index b893f5db2c5b4076ce9a7dfcc08eab8ee867b3ee..de2533c051a5c88c2728e5ff4e66dd85a1f467fa 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -5547,7 +5547,7 @@ static void h2_detach(struct sedesc *sd)
                                         */
                                        HA_ATOMIC_OR(&h2c->wait_event.tasklet->state, TASK_F_USR1);
                                        if (session_check_idle_conn(h2c->conn->owner, h2c->conn) != 0) {
-                                               /* At this point either the connection is destroyed, or it's been added to the server idle list, just stop */
+                                               h2c->conn->mux->destroy(h2c);
                                                TRACE_DEVEL("leaving without reusable idle connection", H2_EV_STRM_END);
                                                return;
                                        }

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 2faa3cc1c4b6de053be842a1fc410a2c4b76205f..093b71fe4ee459ecd0668ddcc42ef1a9d446b4c6 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -3784,24 +3784,24 @@ static void qmux_strm_detach(struct sedesc *sd)
                if (conn->flags & CO_FL_PRIVATE) {
                        TRACE_DEVEL("handle private connection reuse", QMUX_EV_STRM_END, conn);
 
-                       /* Add connection into session. If an error occured,
-                        * conn will be closed if idle, or insert will be
-                        * retried on next detach.
+                       /* Ensure conn is attached into session. Most of the times
+                        * this is already done during connect so this is a no-op.
                         */
                        if (!session_add_conn(sess, conn)) {
                                TRACE_ERROR("error during connection insert into session list", QMUX_EV_STRM_END, conn);
                                conn->owner = NULL;
+
+                               /* Session insert failure. Close conn if idle,
+                                * else insert will be retry on next detach.
+                                */
                                if (!qcc->nb_sc)
                                        goto release;
                        }
 
-                       /* If conn is idle, check if session can keep it. Conn is freed if this is not the case.
-                        * TODO graceful shutdown should be preferable instead of plain mux->destroy().
-                        */
+                       /* If conn is idle, check if session can keep it. Conn is closed if this is not the case. */
                        if (!qcc->nb_sc && session_check_idle_conn(sess, conn)) {
-                               TRACE_DEVEL("idle conn rejected by session", QMUX_EV_STRM_END);
-                               conn = NULL;
-                               goto end;
+                               TRACE_DEVEL("idle conn rejected by session", QMUX_EV_STRM_END, conn);
+                               goto release;
                        }
                }
                else {

diff --git a/src/mux_spop.c b/src/mux_spop.c
index 622515b37864a2e72bd444a6c193f1f760d1494a..a9bc0aab872387aef6ab434e89a8e1c3ebdaee19 100644 (file)
--- a/src/mux_spop.c
+++ b/src/mux_spop.c
@@ -2991,7 +2991,7 @@ static void spop_detach(struct sedesc *sd)
                                 */
                                HA_ATOMIC_OR(&spop_conn->wait_event.tasklet->state, TASK_F_USR1);
                                if (session_check_idle_conn(spop_conn->conn->owner, spop_conn->conn) != 0) {
-                                       /* At this point either the connection is destroyed, or it's been added to the server idle list, just stop */
+                                       spop_conn->conn->mux->destroy(spop_conn);
                                        TRACE_DEVEL("leaving without reusable idle connection", SPOP_EV_STRM_END);
                                        return;
                                }