From: Willy Tarreau Date: Mon, 20 Apr 2026 09:40:08 +0000 (+0200) Subject: MEDIUM: muxes: always set conn->owner to the session that owns the connection X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=90b2154d930ea8985d0083c703bff2c46171dd41;p=thirdparty%2Fhaproxy.git MEDIUM: muxes: always set conn->owner to the session that owns the connection When an idle connection is private or considered private, session_add_conn() is called to add it to the list of connections owned by the session. But in case of allocation failure, the session is not set, which results in a long list of possible situations that are all corner cases which are difficult to test (and debug). This commit relies on the fact that it is already permitted to have conn->owner pointing to a session even if the connection couldn't be added to the session's list, as this was already the case in conn_backend_get() when dealing with HOL_RISK. Also as seen in commit 3aab17bd566 added in 2.4, it is already possible to have conn->owner set with the connection not being in a list, and only the list element is checked for this. This commit modifies session_add_conn() to always set conn->onwer, even if the list element couldn't be allocated. This way it's possible to always refer to conn->owner to find the session owning a private conn even in case of failure to allocate an entry. This requires to change the checks on conn->owner to a check of the list element to see if the connection belongs to a session, the pre-assignment of sess to conn->owner in conn_backend_get() is no longer needed, same for the pre-assignment in http_wait_for_response(), and that's all. The H1 mux remained unchanged because since it cannot multiplex, in case it fails to allocate a pconn, it instantly kills the connection. --- diff --git a/src/backend.c b/src/backend.c index 04992e048..7a1892f97 100644 --- a/src/backend.c +++ b/src/backend.c @@ -1438,7 +1438,6 @@ check_tgid: if (reuse_mode == PR_O_REUSE_SAFE && conn->mux->flags & MX_FL_HOL_RISK) { /* attach the connection to the session private list */ - conn->owner = sess; session_add_conn(sess, conn); } else { diff --git a/src/http_ana.c b/src/http_ana.c index e3964eb76..0751c5005 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -1691,7 +1691,6 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) if ((ctx.value.len >= 4 && strncasecmp(ctx.value.ptr, "Nego", 4) == 0) || (ctx.value.len >= 4 && strncasecmp(ctx.value.ptr, "NTLM", 4) == 0)) { sess->flags |= SESS_FL_PREFER_LAST; - conn_set_owner(srv_conn, sess, NULL); conn_set_private(srv_conn); /* If it fail now, the same will be done in mux->detach() callback */ session_add_conn(srv_conn->owner, srv_conn); diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c index cc183ca9a..b2a254418 100644 --- a/src/mux_fcgi.c +++ b/src/mux_fcgi.c @@ -3747,11 +3747,10 @@ static void fcgi_detach(struct sedesc *sd) (fconn->flags & FCGI_CF_KEEP_CONN)) { if (fconn->conn->flags & CO_FL_PRIVATE) { /* Add the connection in the session serverlist, if not already done */ - if (!session_add_conn(sess, fconn->conn)) - fconn->conn->owner = NULL; + session_add_conn(sess, fconn->conn); if (eb_is_empty(&fconn->streams_by_id)) { - if (!fconn->conn->owner) { + if (!LIST_INLIST(&fconn->conn->sess_el)) { /* Session insertion above has failed and connection is idle, remove it. */ CALL_MUX_NO_RET(fconn->conn->mux, destroy(fconn)); TRACE_DEVEL("outgoing connection killed", FCGI_EV_STRM_END|FCGI_EV_FCONN_ERR); diff --git a/src/mux_h2.c b/src/mux_h2.c index aefa8f785..e8aeaca46 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -5732,11 +5732,10 @@ static void h2_detach(struct sedesc *sd) if (h2c->conn->flags & CO_FL_PRIVATE) { /* Add the connection in the session server list, if not already done */ - if (!session_add_conn(sess, h2c->conn)) - h2c->conn->owner = NULL; + session_add_conn(sess, h2c->conn); if (eb_is_empty(&h2c->streams_by_id)) { - if (!h2c->conn->owner) { + if (!LIST_INLIST(&h2c->conn->sess_el)) { /* Session insertion above has failed and connection is idle, remove it. */ CALL_MUX_NO_RET(h2c->conn->mux, destroy(h2c)); TRACE_DEVEL("leaving on error after killing outgoing connection", H2_EV_STRM_END|H2_EV_H2C_ERR); diff --git a/src/mux_quic.c b/src/mux_quic.c index 68d974ad1..0b0040d9c 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -4101,13 +4101,10 @@ static void qmux_strm_detach(struct sedesc *sd) /* 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_add_conn(sess, conn); if (!qcc->nb_sc) { - if (!conn->owner) { + if (!LIST_INLIST(&conn->sess_el)) { /* Session insertion above has failed and connection is idle, remove it. */ goto release; } diff --git a/src/mux_spop.c b/src/mux_spop.c index e43f8d80a..53f063aa9 100644 --- a/src/mux_spop.c +++ b/src/mux_spop.c @@ -3007,11 +3007,10 @@ static void spop_detach(struct sedesc *sd) if (!(spop_conn->flags & (SPOP_CF_RCVD_SHUT|SPOP_CF_ERR_PENDING|SPOP_CF_ERROR))) { if (spop_conn->conn->flags & CO_FL_PRIVATE) { /* Add the connection in the session server list, if not already done */ - if (!session_add_conn(sess, spop_conn->conn)) - spop_conn->conn->owner = NULL; + session_add_conn(sess, spop_conn->conn); if (eb_is_empty(&spop_conn->streams_by_id)) { - if (!spop_conn->conn->owner) { + if (!LIST_INLIST(&spop_conn->conn->sess_el)) { /* Session insertion above has failed and connection is idle, remove it. */ CALL_MUX_NO_RET(spop_conn->conn->mux, destroy(spop_conn)); TRACE_DEVEL("leaving on error after killing outgoing connection", SPOP_EV_STRM_END|SPOP_EV_SPOP_CONN_ERR); diff --git a/src/session.c b/src/session.c index f53cceb64..1f2cf219d 100644 --- a/src/session.c +++ b/src/session.c @@ -683,12 +683,19 @@ int session_add_conn(struct session *sess, struct connection *conn) HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); - /* Already attach to the session */ + /* Already attached to the session */ if (!LIST_ISEMPTY(&conn->sess_el)) { ret = 1; goto out; } + /* Ensure owner is set for connection. It could have been reset upon a + * session_add_conn() failure. We want to set it even if allocation + * below fails so that the session that is expected to manage this + * connection is always known. + */ + conn->owner = sess; + pconns = sess_get_sess_conns(sess, conn->target); if (!pconns) { pconns = sess_alloc_sess_conns(sess, conn->target); @@ -697,10 +704,6 @@ int session_add_conn(struct session *sess, struct connection *conn) } LIST_APPEND(&pconns->conn_list, &conn->sess_el); - /* Ensure owner is set for connection. It could have been reset - * prior on after a session_add_conn() failure. - */ - conn->owner = sess; ret = 1; out: