MINOR: muxes: enforce thread-safety for private idle conns

When a backend connnection becomes idle, muxes must activate some
protection to mark future access on it as dangerous. Indeed, once a
connection is inserted in an idle list, it may be manipulated by another
thread, either via takeover or scheduled for purging.

Private idle connections are stored into a session instead of the server
tree. They are never subject to a takeover for reuse or purge mechanism.
As such, currently they do not require the same level of protection.

However, a new patch will introduce support for private idle connections
purging. Thus, the purpose of this patch is to ensure protection is
activated as well now.

TASK_F_USR1 was already set on them as an anticipation for such need.
Only some extra operations were missing, most notably xprt_set_idle()
invokation. Also, return path of muxes detach operation is adjusted to
ensure such connection are never accessed after insertion.

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index 949ef19c1cf3bffd156a5a991acf0bc9f503dc05..c0778f44c4a33a65380f2ca9516d85cd9d96ffc6 100644 (file)
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -3748,6 +3748,7 @@ static void fcgi_detach(struct sedesc *sd)
                                 * that the handler needs to check it under the idle conns lock.
                                 */
                                HA_ATOMIC_OR(&fconn->wait_event.tasklet->state, TASK_F_USR1);
+                               xprt_set_idle(fconn->conn, fconn->conn->xprt, fconn->conn->xprt_ctx);
 
                                /* Ensure session can keep a new idle connection. */
                                if (session_check_idle_conn(sess, fconn->conn) != 0) {
@@ -3755,6 +3756,13 @@ static void fcgi_detach(struct sedesc *sd)
                                        TRACE_DEVEL("outgoing connection killed", FCGI_EV_STRM_END|FCGI_EV_FCONN_ERR);
                                        return;
                                }
+
+                               /* At this point, the connection is inserted into
+                                * session list and marked as idle, so it may already
+                                * have been purged from another thread.
+                                */
+                               TRACE_DEVEL("private connection marked as idle", FCGI_EV_STRM_END, fconn->conn);
+                               return;
                        }
                }
                else {

diff --git a/src/mux_h1.c b/src/mux_h1.c
index 8aeec3931f21f15f2c4dae29d9fd9a6fbb294a17..86ca866b4b84e262da39fe89d817d12cac761b65 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -1151,6 +1151,8 @@ static int h1s_finish_detach(struct h1s *h1s)
                         * that the handler needs to check it under the idle conns lock.
                         */
                        HA_ATOMIC_OR(&h1c->wait_event.tasklet->state, TASK_F_USR1);
+                       h1c->conn->xprt->subscribe(h1c->conn, h1c->conn->xprt_ctx, SUB_RETRY_RECV, &h1c->wait_event);
+                       xprt_set_idle(h1c->conn, h1c->conn->xprt, h1c->conn->xprt_ctx);
 
                        /* Ensure session can keep a new idle connection. */
                        if (session_check_idle_conn(sess, h1c->conn)) {
@@ -1158,6 +1160,12 @@ static int h1s_finish_detach(struct h1s *h1s)
                                h1c->conn->mux->destroy(h1c);
                                goto released;
                        }
+
+                       /* At this point, the connection is inserted into
+                        * session list and marked as idle, so it may already
+                        * have been purged from another thread.
+                        */
+                       goto end;
                }
                else {
                        if (h1c->conn->owner == sess)

diff --git a/src/mux_h2.c b/src/mux_h2.c
index bf678e26e8db3e972e41771b9fc70cf62c4c863e..57fea0b5a2c286c6f4d6114b289a34898d21df1f 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -5558,6 +5558,7 @@ static void h2_detach(struct sedesc *sd)
                                         * that the handler needs to check it under the idle conns lock.
                                         */
                                        HA_ATOMIC_OR(&h2c->wait_event.tasklet->state, TASK_F_USR1);
+                                       xprt_set_idle(h2c->conn, h2c->conn->xprt, h2c->conn->xprt_ctx);
 
                                        /* Ensure session can keep a new idle connection. */
                                        if (session_check_idle_conn(sess, h2c->conn) != 0) {
@@ -5565,6 +5566,13 @@ static void h2_detach(struct sedesc *sd)
                                                TRACE_DEVEL("leaving without reusable idle connection", H2_EV_STRM_END);
                                                return;
                                        }
+
+                                       /* At this point, the connection is inserted into
+                                        * session list and marked as idle, so it may already
+                                        * have been purged from another thread.
+                                        */
+                                       TRACE_DEVEL("private connection marked as idle", H2_EV_STRM_END);
+                                       return;
                                }
                        }
                        else {

diff --git a/src/mux_spop.c b/src/mux_spop.c
index 77ee3a2425455098b11eab06a164073d4147654f..4ba8dabcad3563047194fdd3cba3ae2db8ea97a0 100644 (file)
--- a/src/mux_spop.c
+++ b/src/mux_spop.c
@@ -3005,6 +3005,7 @@ static void spop_detach(struct sedesc *sd)
                                 * that the handler needs to check it under the idle conns lock.
                                 */
                                HA_ATOMIC_OR(&spop_conn->wait_event.tasklet->state, TASK_F_USR1);
+                               xprt_set_idle(spop_conn->conn, spop_conn->conn->xprt, spop_conn->conn->xprt_ctx);
 
                                /* Ensure session can keep a new idle connection. */
                                if (session_check_idle_conn(sess, spop_conn->conn) != 0) {
@@ -3012,6 +3013,13 @@ static void spop_detach(struct sedesc *sd)
                                        TRACE_DEVEL("leaving without reusable idle connection", SPOP_EV_STRM_END);
                                        return;
                                }
+
+                               /* At this point, the connection is inserted into
+                                * session list and marked as idle, so it may already
+                                * have been purged from another thread.
+                                */
+                               TRACE_DEVEL("private connection marked as idle", SPOP_EV_STRM_END);
+                               return;
                        }
                }
                else {