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 04992e048d4330c30f1e99128f37d7b8ca1ab489..7a1892f971ae40526eb5329d37fd3c7876cd2c7f 100644 (file)
--- 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 e3964eb760d6e6c27b6251cdcc4dc74a1b50626f..0751c50050f6e9e9ab2bb2ac89240796d2fbef97 100644 (file)
--- 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 cc183ca9adfe8d8c45350411ffbdb04030b1a627..b2a254418318c46c67d9af62d13569e2d4a01309 100644 (file)
--- 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 aefa8f7853238ef63ca21fb871dd696b1f2e1964..e8aeaca468729130e21ea7b72d986d5da00d88e7 100644 (file)
--- 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 68d974ad123f0300ee4463ecbb0c3f669b2babd7..0b0040d9c00ceca1dd3a2f6aed918466a11eccef 100644 (file)
--- 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 e43f8d80af43121190f48719f08b02c44761503d..53f063aa9d59ac98828473e202bf3b9c2b0101a6 100644 (file)
--- 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 f53cceb64b1bf76f5e0cad81d043b7d47b2c7209..1f2cf219d71bc2b835ad8736978ef97d68d2e9be 100644 (file)
--- 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: