BUG/MAJOR: server: do not delete srv referenced by session

A server can only be deleted if there is no elements which reference it.
This is taken care via srv_check_for_deletion(), most notably for active
and idle connections.

A special case occurs for connections directly managed by a session.
This is for so-called private connections, when using http-reuse never
or H2 + http-reuse safe for example. In this case. server does not
account these connections into its idle lists. This caused a bug as the
server is deleted despite the session still being able to access it.

To properly fix this, add a new referencing element into the server for
these session connections. A mt_list has been chosen for this. On
default http-reuse, private connections are typically not used so it
won't make any difference. If using H2 servers, or more generally when
dealing with private connections, insert/delete should typically occur
only once per session lifetime so impact on performance should be
minimal.

This should be backported up to 2.4. Note that srv_check_for_deletion()
was introduced in 3.0 dev tree. On backport, the extra condition in it
should be placed in cli_parse_delete_server() instead.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index f077ff2f887c0878aaa9a545cfcc08957948d524..c2c3238ef4cca57d9dc670914b98299240005264 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -338,6 +338,7 @@ struct server {
        unsigned int est_need_conns;            /* Estimate on the number of needed connections (max of curr and previous max_used) */
 
        struct queue queue;                     /* pending connections */
+       struct mt_list sess_conns;              /* list of private conns managed by a session on this server */
 
        /* Element below are usd by LB algorithms and must be doable in
         * parallel to other threads reusing connections above.

diff --git a/include/haproxy/session-t.h b/include/haproxy/session-t.h
index bf0079129a82e5b907cdd49b280d07836d11bf4b..aa2bf1e82f55406745d7097f831b2804835b5ba8 100644 (file)
--- a/include/haproxy/session-t.h
+++ b/include/haproxy/session-t.h
@@ -62,11 +62,16 @@ struct session {
        struct sockaddr_storage *dst; /* destination address (pool), when known, otherwise NULL */
 };
 
-/* List of private conns managed by a session, indexed by server */
+/*
+ * List of private conns managed by a session, indexed by server
+ * Stored both into the session and server instances
+ */
 struct sess_priv_conns {
        void *target;                   /* Server or dispatch used for indexing */
        struct list conn_list;          /* Head of the connections list */
+
        struct list sess_el;            /* Element of session.priv_conns */
+       struct mt_list srv_el;          /* Element of server.sess_conns */
 };
 
 #endif /* _HAPROXY_SESSION_T_H */

diff --git a/include/haproxy/session.h b/include/haproxy/session.h
index 90970e8e285288744a90dd4eb77a91719056aa29..0a73e74c3c88b81e366517c3f06151bf8674b61e 100644 (file)
--- a/include/haproxy/session.h
+++ b/include/haproxy/session.h
@@ -161,6 +161,7 @@ static inline void session_unown_conn(struct session *sess, struct connection *c
                if (pconns->target == conn->target) {
                        if (LIST_ISEMPTY(&pconns->conn_list)) {
                                LIST_DELETE(&pconns->sess_el);
+                               MT_LIST_DELETE(&pconns->srv_el);
                                pool_free(pool_head_sess_priv_conns, pconns);
                        }
                        break;
@@ -176,6 +177,7 @@ static inline void session_unown_conn(struct session *sess, struct connection *c
 static inline int session_add_conn(struct session *sess, struct connection *conn, void *target)
 {
        struct sess_priv_conns *pconns = NULL;
+       struct server *srv = objt_server(conn->target);
        int found = 0;
 
        BUG_ON(objt_listener(conn->target));
@@ -198,6 +200,10 @@ static inline int session_add_conn(struct session *sess, struct connection *conn
                pconns->target = target;
                LIST_INIT(&pconns->conn_list);
                LIST_APPEND(&sess->priv_conns, &pconns->sess_el);
+
+               MT_LIST_INIT(&pconns->srv_el);
+               if (srv)
+                       MT_LIST_APPEND(&srv->sess_conns, &pconns->srv_el);
        }
        LIST_APPEND(&pconns->conn_list, &conn->sess_el);
        return 1;

diff --git a/src/server.c b/src/server.c
index 1450b0477accf4e6a0ca66ef96235701eb4d6dd8..e22b33806c1dca639945015394150f043a8768b2 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -2819,6 +2819,8 @@ struct server *new_server(struct proxy *proxy)
        srv->agent.proxy = proxy;
        srv->xprt  = srv->check.xprt = srv->agent.xprt = xprt_get(XPRT_RAW);
 
+       MT_LIST_INIT(&srv->sess_conns);
+
        srv->extra_counters = NULL;
 #ifdef USE_OPENSSL
        HA_RWLOCK_INIT(&srv->ssl_ctx.lock);
@@ -5832,6 +5834,7 @@ int srv_check_for_deletion(const char *bename, const char *svname, struct proxy
         * cleanup function should be implemented to be used here.
         */
        if (srv->curr_used_conns || srv->curr_idle_conns ||
+           !MT_LIST_ISEMPTY(&srv->sess_conns) ||
            !eb_is_empty(&srv->queue.head) || srv_has_streams(srv)) {
                msg = "Server still has connections attached to it, cannot remove it.";
                goto leave;

diff --git a/src/session.c b/src/session.c
index f54490e79f49cb2afb18c505cdf551aa3d895703..55e084b5d7263170c99489110d28f6a1e150270c 100644 (file)
--- a/src/session.c
+++ b/src/session.c
@@ -102,6 +102,7 @@ void session_free(struct session *sess)
                                conn_free(conn);
                        }
                }
+               MT_LIST_DELETE(&pconns->srv_el);
                pool_free(pool_head_sess_priv_conns, pconns);
        }
        sockaddr_free(&sess->src);