From a889413f5ec5e2c0c6ed30debfd16f666dbc0f99 Mon Sep 17 00:00:00 2001 From: Oliver Dala Date: Wed, 25 Sep 2024 11:37:25 +0200 Subject: [PATCH] BUG/MEDIUM: cli: Deadlock when setting frontend maxconn The proxy lock state isn't passed down to relax_listener through dequeue_proxy_listeners, which causes a deadlock in relax_listener when it tries to get that lock. Backporting: Older versions didn't have relax_listener and directly called resume_listener in dequeue_proxy_listeners. lpx should just be passed directly to resume_listener then. The bug was introduced in commit 001328873c352e5e4b1df0dcc8facaf2fc1408aa [cf: This patch should fix the issue #2726. It must be backported as far as 2.4] --- include/haproxy/listener.h | 2 +- src/listener.c | 14 +++++++++----- src/proxy.c | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/include/haproxy/listener.h b/include/haproxy/listener.h index 3627a791e..fb32f2e89 100644 --- a/include/haproxy/listener.h +++ b/include/haproxy/listener.h @@ -94,7 +94,7 @@ void enable_listener(struct listener *listener); void dequeue_all_listeners(void); /* Dequeues all listeners waiting for a resource in proxy 's queue */ -void dequeue_proxy_listeners(struct proxy *px); +void dequeue_proxy_listeners(struct proxy *px, int lpx); /* This function closes the listening socket for the specified listener, * provided that it's already in a listening state. The listener enters the diff --git a/src/listener.c b/src/listener.c index d32551a24..10ee800f4 100644 --- a/src/listener.c +++ b/src/listener.c @@ -707,8 +707,12 @@ void dequeue_all_listeners() } } -/* Dequeues all listeners waiting for a resource in proxy 's queue */ -void dequeue_proxy_listeners(struct proxy *px) +/* Dequeues all listeners waiting for a resource in proxy 's queue + * The caller is responsible for indicating in lpx, whether the proxy's lock + * is already held (non-zero) or not (zero) so that this information can be + * passed to relax_listener +*/ +void dequeue_proxy_listeners(struct proxy *px, int lpx) { struct listener *listener; @@ -716,7 +720,7 @@ void dequeue_proxy_listeners(struct proxy *px) /* This cannot fail because the listeners are by definition in * the LI_LIMITED state. */ - relax_listener(listener, 0, 0); + relax_listener(listener, lpx, 0); } } @@ -1558,7 +1562,7 @@ void listener_accept(struct listener *l) if (p && !MT_LIST_ISEMPTY(&p->listener_queue) && (!p->fe_sps_lim || freq_ctr_remain(&p->fe_counters.sess_per_sec, p->fe_sps_lim, 0) > 0)) - dequeue_proxy_listeners(p); + dequeue_proxy_listeners(p, 0); } return; @@ -1617,7 +1621,7 @@ void listener_release(struct listener *l) if (fe && !MT_LIST_ISEMPTY(&fe->listener_queue) && (!fe->fe_sps_lim || freq_ctr_remain(&fe->fe_counters.sess_per_sec, fe->fe_sps_lim, 0) > 0)) - dequeue_proxy_listeners(fe); + dequeue_proxy_listeners(fe, 0); else { unsigned int wait; int expire = TICK_ETERNITY; diff --git a/src/proxy.c b/src/proxy.c index 77cfe8654..33c3be22d 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -2030,7 +2030,7 @@ struct task *manage_proxy(struct task *t, void *context, unsigned int state) } /* The proxy is not limited so we can re-enable any waiting listener */ - dequeue_proxy_listeners(p); + dequeue_proxy_listeners(p, 0); out: t->expire = next; task_queue(t); @@ -3041,7 +3041,7 @@ static int cli_parse_set_maxconn_frontend(char **args, char *payload, struct app } if (px->maxconn > px->feconn) - dequeue_proxy_listeners(px); + dequeue_proxy_listeners(px, 1); HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);