BUG/MAJOR: backend: fix idle conn crash under low FD

Since the following commit, idle conns are stored in a list as secondary
storage to retrieve them in usage order :
  5afcb686b93c3811bd859a331efd6a8341a61218
  MAJOR: connection: purge idle conn by last usage

The list usage has been extended wherever connections lookup are done
both on idle and safe trees. This reduced the code size by replacing a
two tree loops by a single list loop.

LIST_ELEM() is used in this context to retrieve the first idle list
element from the server list head. However, macro usage was wrong due to
an extra '&' operator which returns an invalid connection reference.
This will most of the time caused a crash on conn_delete_from_tree() or
affiliated functions.

This bug only occurs if the FD pool is exhausted and some idle
connections are selected to be killed.

It can be reproduced using the following config and h2load command :
$ h2load -t 8 -c 800 -m 10 -n 800 "http://127.0.0.1:21080/?s=10k"

global
	maxconn 100

defaults
	mode http
	timeout connect 20s
	timeout client  20s
	timeout server  20s

listen li
	bind :21080 proto h2
	server nginx 127.99.0.1:30080 proto h1

This bug has been introduced by the above commit. Thus no need to
backport this fix.

Note that LIST_ELEM() macro usage was slightly adjusted also in
srv_migrate_conns_to_remove(). The function used toremove_list instead
of idle_list connection list element. This is not a bug as they are
stored in the same union. However, the new code is clearer as it intends
to move connection from the idle_list only into the toremove_list
mt-list.

diff --git a/src/backend.c b/src/backend.c
index c6317bb261aff890b7403d3f869f65ca2aef8d68..359bd499fedc6c9e6bea8bb69081b9343516efe9 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -1515,7 +1515,7 @@ static int connect_server(struct stream *s)
                /* First, try from our own idle list */
                HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
                if (!LIST_ISEMPTY(&srv->per_thr[tid].idle_conn_list)) {
-                       tokill_conn = LIST_ELEM(&srv->per_thr[tid].idle_conn_list.n, struct connection *, idle_list);
+                       tokill_conn = LIST_ELEM(srv->per_thr[tid].idle_conn_list.n, struct connection *, idle_list);
                        conn_delete_from_tree(tokill_conn);
                        HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 
@@ -1544,7 +1544,7 @@ static int connect_server(struct stream *s)
                                        continue;
 
                                if (!LIST_ISEMPTY(&srv->per_thr[i].idle_conn_list)) {
-                                       tokill_conn = LIST_ELEM(&srv->per_thr[i].idle_conn_list.n, struct connection *, idle_list);
+                                       tokill_conn = LIST_ELEM(srv->per_thr[i].idle_conn_list.n, struct connection *, idle_list);
                                        conn_delete_from_tree(tokill_conn);
                                }
                                HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock);

diff --git a/src/server.c b/src/server.c
index e7ce3d70b2899100fa5fd957b51ce69b3ca75e13..37e8beba0b802513052e13c649db29033776ae6f 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -5949,7 +5949,7 @@ static int srv_migrate_conns_to_remove(struct list *list, struct mt_list *toremo
                if (toremove_nb != -1 && i >= toremove_nb)
                        break;
 
-               conn = LIST_ELEM(list->n, struct connection *, toremove_list);
+               conn = LIST_ELEM(list->n, struct connection *, idle_list);
                conn_delete_from_tree(conn);
                MT_LIST_APPEND(toremove_list, &conn->toremove_list);
                i++;