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.
(cherry picked from commit 7dae3ceaa08ead56104bae6471b236de536c3f51)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 2b9328a831f4343b29b0810546f12b850ffd3fb8)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index e3bcfc3..40a5278 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -318,6 +318,7 @@
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 bf00791..aa2bf1e 100644
--- a/include/haproxy/session-t.h
+++ b/include/haproxy/session-t.h
@@ -62,11 +62,16 @@
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 69a0333..1bfb01d 100644
--- a/include/haproxy/session.h
+++ b/include/haproxy/session.h
@@ -151,6 +151,7 @@
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;
@@ -166,6 +167,7 @@
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));
@@ -188,6 +190,10 @@
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);
diff --git a/src/server.c b/src/server.c
index 7796d1d..25ecf53 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2507,6 +2507,8 @@
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);
@@ -5187,6 +5189,7 @@
* 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)) {
cli_err(appctx, "Server still has connections attached to it, cannot remove it.");
goto out;
diff --git a/src/session.c b/src/session.c
index 10ed310..d46c0f5 100644
--- a/src/session.c
+++ b/src/session.c
@@ -102,6 +102,7 @@
conn_free(conn);
}
}
+ MT_LIST_DELETE(&pconns->srv_el);
pool_free(pool_head_sess_priv_conns, pconns);
}
sockaddr_free(&sess->src);