Skip to content

Commit

Permalink
BUG/MEDIUM: cli: Deadlock when setting frontend maxconn
Browse files Browse the repository at this point in the history
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 0013288

[cf: This patch should fix the issue haproxy#2726. It must be backported as far as
2.4]
  • Loading branch information
Olliferdl authored and capflam committed Sep 25, 2024
1 parent 96edacc commit a889413
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 8 deletions.
2 changes: 1 addition & 1 deletion include/haproxy/listener.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void enable_listener(struct listener *listener);
void dequeue_all_listeners(void);

/* Dequeues all listeners waiting for a resource in proxy <px>'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
Expand Down
14 changes: 9 additions & 5 deletions src/listener.c
Original file line number Diff line number Diff line change
Expand Up @@ -707,16 +707,20 @@ void dequeue_all_listeners()
}
}

/* Dequeues all listeners waiting for a resource in proxy <px>'s queue */
void dequeue_proxy_listeners(struct proxy *px)
/* Dequeues all listeners waiting for a resource in proxy <px>'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;

while ((listener = MT_LIST_POP(&px->listener_queue, struct listener *, wait_queue))) {
/* This cannot fail because the listeners are by definition in
* the LI_LIMITED state.
*/
relax_listener(listener, 0, 0);
relax_listener(listener, lpx, 0);
}
}

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/proxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit a889413

Please sign in to comment.