From: Amaury Denoyelle Date: Thu, 24 Jul 2025 09:29:50 +0000 (+0200) Subject: MINOR: session: do not release conn in session_check_idle_conn() X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dd9645d6b9fd48e81bb25ec6f64f86fabcbd8924;p=thirdparty%2Fhaproxy.git 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 8ec1f7df5..6b1824aba 100644 --- 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 is able to keep idle connection . 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 ce607422b..f91abf207 100644 --- 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 1e9c2b0de..a986483ef 100644 --- 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 b893f5db2..de2533c05 100644 --- 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 2faa3cc1c..093b71fe4 100644 --- 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 622515b37..a9bc0aab8 100644 --- 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; }