From 697f7d1142a26352940532d0481cc17f9225d0d1 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Wed, 30 Jul 2025 16:13:42 +0200 Subject: [PATCH] MINOR: muxes: refactor private connection detach Following the latest adjustment on session_add_conn() / session_check_idle_conn(), detach muxes callbacks were rewritten for private connection handling. Nothing really fancy here : some more explicit comments and the removal of a duplicate checks on idle conn status for muxes with true multipexing support. --- src/mux_fcgi.c | 15 +++++++++------ src/mux_h1.c | 6 +++++- src/mux_h2.c | 14 +++++++++----- src/mux_quic.c | 19 ++++++++++--------- src/mux_spop.c | 14 +++++++++----- 5 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c index f91abf207..33f7ef0ab 100644 --- a/src/mux_fcgi.c +++ b/src/mux_fcgi.c @@ -3723,21 +3723,24 @@ 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)) { + if (!session_add_conn(sess, fconn->conn)) fconn->conn->owner = NULL; - if (eb_is_empty(&fconn->streams_by_id)) { - /* let's kill the connection right away */ + + if (eb_is_empty(&fconn->streams_by_id)) { + if (!fconn->conn->owner) { + /* Session insertion above has failed and connection is idle, remove it. */ fconn->conn->mux->destroy(fconn); TRACE_DEVEL("outgoing connection killed", FCGI_EV_STRM_END|FCGI_EV_FCONN_ERR); return; } - } - if (eb_is_empty(&fconn->streams_by_id)) { + /* mark that the tasklet may lose its context to another thread and * that the handler needs to check it under the idle conns lock. */ HA_ATOMIC_OR(&fconn->wait_event.tasklet->state, TASK_F_USR1); - if (session_check_idle_conn(fconn->conn->owner, fconn->conn) != 0) { + + /* Ensure session can keep a new idle connection. */ + if (session_check_idle_conn(sess, fconn->conn) != 0) { 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 a986483ef..bc2e940c1 100644 --- a/src/mux_h1.c +++ b/src/mux_h1.c @@ -1139,16 +1139,20 @@ static int h1s_finish_detach(struct h1s *h1s) if (h1c->conn->flags & CO_FL_PRIVATE) { /* Add the connection in the session server list, if not already done */ if (!session_add_conn(sess, h1c->conn)) { + /* HTTP/1.1 conn is always idle after detach, can be removed if session insert failed. */ h1c->conn->owner = NULL; h1c->conn->mux->destroy(h1c); goto released; } - /* Always idle at this step */ + + /* HTTP/1.1 conn is always idle after detach. */ /* mark that the tasklet may lose its context to another thread and * that the handler needs to check it under the idle conns lock. */ HA_ATOMIC_OR(&h1c->wait_event.tasklet->state, TASK_F_USR1); + + /* Ensure session can keep a new idle connection. */ if (session_check_idle_conn(sess, h1c->conn)) { TRACE_DEVEL("outgoing connection rejected", H1_EV_STRM_END|H1_EV_H1C_END, h1c->conn); h1c->conn->mux->destroy(h1c); diff --git a/src/mux_h2.c b/src/mux_h2.c index de2533c05..76bea0a61 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -5533,20 +5533,24 @@ 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)) { + if (!session_add_conn(sess, h2c->conn)) h2c->conn->owner = NULL; - if (eb_is_empty(&h2c->streams_by_id)) { + + if (eb_is_empty(&h2c->streams_by_id)) { + if (!h2c->conn->owner) { + /* Session insertion above has failed and connection is idle, remove it. */ h2c->conn->mux->destroy(h2c); TRACE_DEVEL("leaving on error after killing outgoing connection", H2_EV_STRM_END|H2_EV_H2C_ERR); return; } - } - if (eb_is_empty(&h2c->streams_by_id)) { + /* mark that the tasklet may lose its context to another thread and * that the handler needs to check it under the idle conns lock. */ HA_ATOMIC_OR(&h2c->wait_event.tasklet->state, TASK_F_USR1); - if (session_check_idle_conn(h2c->conn->owner, h2c->conn) != 0) { + + /* Ensure session can keep a new idle connection. */ + if (session_check_idle_conn(sess, h2c->conn) != 0) { 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 093b71fe4..64b09d79f 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -3790,18 +3790,19 @@ static void qmux_strm_detach(struct sedesc *sd) 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) + if (!qcc->nb_sc) { + if (!conn->owner) { + /* Session insertion above has failed and connection is idle, remove it. */ goto release; - } + } - /* 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); - goto release; + /* Ensure session can keep a new idle connection. */ + if (session_check_idle_conn(sess, conn)) { + 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 a9bc0aab8..4e376b66d 100644 --- a/src/mux_spop.c +++ b/src/mux_spop.c @@ -2977,20 +2977,24 @@ 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)) { + if (!session_add_conn(sess, spop_conn->conn)) spop_conn->conn->owner = NULL; - if (eb_is_empty(&spop_conn->streams_by_id)) { + + if (eb_is_empty(&spop_conn->streams_by_id)) { + if (!spop_conn->conn->owner) { + /* Session insertion above has failed and connection is idle, remove it. */ 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); return; } - } - if (eb_is_empty(&spop_conn->streams_by_id)) { + /* mark that the tasklet may lose its context to another thread and * that the handler needs to check it under the idle conns lock. */ 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) { + + /* Ensure session can keep a new idle connection. */ + if (session_check_idle_conn(sess, spop_conn->conn) != 0) { spop_conn->conn->mux->destroy(spop_conn); TRACE_DEVEL("leaving without reusable idle connection", SPOP_EV_STRM_END); return; -- 2.47.2