MEDIUM: conn/muxes/ssl: remove BE priv idle conn from sess on IO

This is a direct follow-up of previous patch which adjust idle private
connections access via input/output handlers.

This patch implement the handlers prologue part. Now, private idle
connections require a similar treatment with non-private idle
connections. Thus, private conns are removed temporarily from its
session under protection of idle_conns lock.

As locking usage is already performed in input/output handler,
session_unown_conn() cannot be called. Thus, a new function
session_detach_idle_conn() is implemented in session module, which
performs basically the same operation but relies on external locking.

include/haproxy/session.h

@@ -48,6 +48,7 @@ int session_reinsert_idle_conn(struct session *sess, struct connection *conn);
 int session_check_idle_conn(struct session *sess, struct connection *conn);
 struct connection *session_get_conn(struct session *sess, void *target, int64_t hash);
 void session_unown_conn(struct session *sess, struct connection *conn);
+int session_detach_idle_conn(struct session *sess, struct connection *conn);
 
 /* Remove the refcount from the session to the tracked counters, and clear the
  * pointer to ensure this is only performed once. The caller is responsible for

src/connection.c

@@ -199,12 +199,18 @@ int conn_notify_mux(struct connection *conn, int old_flags, int forced_wake)
 	     ((conn->flags ^ old_flags) & CO_FL_NOTIFY_DONE) ||
 	     ((old_flags & CO_FL_WAIT_XPRT) && !(conn->flags & CO_FL_WAIT_XPRT))) &&
 	    conn->mux && conn->mux->wake) {
-		uint conn_in_list = conn->flags & CO_FL_LIST_MASK;
+		uint conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE);
 		struct server *srv = objt_server(conn->target);
 
 		if (conn_in_list) {
 			HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-			conn_delete_from_tree(conn);
+			if (conn->flags & CO_FL_SESS_IDLE) {
+				if (!session_detach_idle_conn(conn->owner, conn))
+					conn_in_list = 0;
+			}
+			else {
+				conn_delete_from_tree(conn);
+			}
 			HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 		}
 

src/mux_fcgi.c

@@ -3061,9 +3061,16 @@ struct task *fcgi_io_cb(struct task *t, void *ctx, unsigned int state)
 		conn = fconn->conn;
 		TRACE_POINT(FCGI_EV_FCONN_WAKE, conn);
 
-		conn_in_list = conn->flags & CO_FL_LIST_MASK;
+		conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE);
-		if (conn_in_list)
+		if (conn_in_list) {
-			conn_delete_from_tree(conn);
+			if (conn->flags & CO_FL_SESS_IDLE) {
+				if (!session_detach_idle_conn(conn->owner, conn))
+					conn_in_list = 0;
+			}
+			else {
+				conn_delete_from_tree(conn);
+			}
+		}
 
 		HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 	} else {
@@ -3322,6 +3329,8 @@ struct task *fcgi_timeout_task(struct task *t, void *context, unsigned int state
 		 */
 		if (fconn->conn->flags & CO_FL_LIST_MASK)
 			conn_delete_from_tree(fconn->conn);
+		else if (fconn->conn->flags & CO_FL_SESS_IDLE)
+			session_detach_idle_conn(fconn->conn->owner, fconn->conn);
 
 		HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 

src/mux_h1.c

@@ -4306,9 +4306,16 @@ struct task *h1_io_cb(struct task *t, void *ctx, unsigned int state)
 		/* Remove the connection from the list, to be sure nobody attempts
 		 * to use it while we handle the I/O events
 		 */
-		conn_in_list = conn->flags & CO_FL_LIST_MASK;
+		conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE);
-		if (conn_in_list)
+		if (conn_in_list) {
-			conn_delete_from_tree(conn);
+			if (conn->flags & CO_FL_SESS_IDLE) {
+				if (!session_detach_idle_conn(conn->owner, conn))
+					conn_in_list = 0;
+			}
+			else {
+				conn_delete_from_tree(conn);
+			}
+		}
 
 		HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 	} else {

src/mux_h2.c

@@ -4962,9 +4962,16 @@ struct task *h2_io_cb(struct task *t, void *ctx, unsigned int state)
 		/* Remove the connection from the list, to be sure nobody attempts
 		 * to use it while we handle the I/O events
 		 */
-		conn_in_list = conn->flags & CO_FL_LIST_MASK;
+		conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE);
-		if (conn_in_list)
+		if (conn_in_list) {
-			conn_delete_from_tree(conn);
+			if (conn->flags & CO_FL_SESS_IDLE) {
+				if (!session_detach_idle_conn(conn->owner, conn))
+					conn_in_list = 0;
+			}
+			else {
+				conn_delete_from_tree(conn);
+			}
+		}
 
 		HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 	} else {
@@ -5245,6 +5252,8 @@ struct task *h2_timeout_task(struct task *t, void *context, unsigned int state)
 		 */
 		if (h2c->conn->flags & CO_FL_LIST_MASK)
 			conn_delete_from_tree(h2c->conn);
+		else if (h2c->conn->flags & CO_FL_SESS_IDLE)
+			session_detach_idle_conn(h2c->conn->owner, h2c->conn);
 
 		HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 

src/mux_spop.c

@@ -2557,9 +2557,16 @@ static struct task *spop_io_cb(struct task *t, void *ctx, unsigned int state)
 		conn = spop_conn->conn;
 		TRACE_POINT(SPOP_EV_SPOP_CONN_WAKE, conn);
 
-		conn_in_list = conn->flags & CO_FL_LIST_MASK;
+		conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE);
-		if (conn_in_list)
+		if (conn_in_list) {
-			conn_delete_from_tree(conn);
+			if (conn->flags & CO_FL_SESS_IDLE) {
+				if (!session_detach_idle_conn(conn->owner, conn))
+					conn_in_list = 0;
+			}
+			else {
+				conn_delete_from_tree(conn);
+			}
+		}
 
 		HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 	} else {
@@ -2792,6 +2799,8 @@ static struct task *spop_timeout_task(struct task *t, void *context, unsigned in
 		 */
 		if (spop_conn->conn->flags & CO_FL_LIST_MASK)
 			conn_delete_from_tree(spop_conn->conn);
+		else if (spop_conn->conn->flags & CO_FL_SESS_IDLE)
+			session_detach_idle_conn(spop_conn->conn->owner, spop_conn->conn);
 
 		HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 

src/session.c

@@ -837,6 +837,43 @@ void session_unown_conn(struct session *sess, struct connection *conn)
 	HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 }
 
+/* Remove <conn> connection from <sess> session. Contrary to
+ * session_unown_conn(), this function is not protected by a lock, so the
+ * caller is responsible to properly use idle_conns_lock prior to calling it.
+ *
+ * Another notable difference is that <owner> member of <conn> is not resetted.
+ * This is a convenience as this function usage is generally coupled with a
+ * following session_reinsert_idle_conn().
+ *
+ * Must be called with idle_conns_lock held.
+ *
+ * Returns true on connection removal, false if it was already not stored.
+ */
+int session_detach_idle_conn(struct session *sess, struct connection *conn)
+{
+	struct sess_priv_conns *pconns;
+
+	if (!LIST_INLIST(&conn->sess_el))
+		return 0;
+
+	/* This function is reserved for idle private connections. */
+	BUG_ON(!(conn->flags & CO_FL_SESS_IDLE));
+
+	--sess->idle_conns;
+	LIST_DEL_INIT(&conn->sess_el);
+
+	pconns = sess_get_sess_conns(sess, conn->target);
+	BUG_ON(!pconns); /* if conn is attached to session, its sess_conn must exists. */
+	if (LIST_ISEMPTY(&pconns->conn_list)) {
+		/* Remove sess_conn element as no connection left in it. */
+		LIST_DELETE(&pconns->sess_el);
+		MT_LIST_DELETE(&pconns->srv_el);
+		pool_free(pool_head_sess_priv_conns, pconns);
+	}
+
+	return 1;
+}
+
 
 /*
  * Local variables:

src/ssl_sock.c

@@ -6403,9 +6403,21 @@ struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned int state)
 		}
 		ctx = context;
 		conn = ctx->conn;
-		conn_in_list = conn->flags & CO_FL_LIST_MASK;
+
-		if (conn_in_list)
+		/* Remove the connection from the list, to be sure nobody attempts
-			conn_delete_from_tree(conn);
+		 * to use it while we handle the I/O events
+		 */
+		conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE);
+		if (conn_in_list) {
+			if (conn->flags & CO_FL_SESS_IDLE) {
+				if (!session_detach_idle_conn(conn->owner, conn))
+					conn_in_list = 0;
+			}
+			else {
+				conn_delete_from_tree(conn);
+			}
+		}
+
 		HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 	} else {
 		ctx = context;