MINOR: listener: small API change

A minor API change was performed in listener(.c/.h) to restore consistency
between stop_listener() and (resume/pause)_listener() functions.

LISTENER_LOCK was never locked prior to calling stop_listener():
lli variable hint is thus not useful anymore.

Added PROXY_LOCK locking in (resume/pause)_listener() functions
with related lpx variable hint (prerequisite for #1626).

It should be backported to 2.6, 2.5 and 2.4

(cherry picked from commit 001328873c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
This commit is contained in:
Aurelien DARRAGON 2022-09-09 15:32:57 +02:00 committed by Christopher Faulet
parent 3c263c09fa
commit 614f99ee0a
4 changed files with 53 additions and 28 deletions

View File

@ -42,23 +42,29 @@ void listener_set_state(struct listener *l, enum li_state st);
* closes upon SHUT_WR and refuses to rebind. So a common validation path
* involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling
* is disabled. It normally returns non-zero, unless an error is reported.
* It will need to operate under the proxy's lock. The caller is
* responsible for indicating in lpx whether the proxy locks is
* already held (non-zero) or not (zero) so that the function picks it.
*/
int pause_listener(struct listener *l);
int pause_listener(struct listener *l, int lpx);
/* This function tries to resume a temporarily disabled listener.
* The resulting state will either be LI_READY or LI_FULL. 0 is returned
* in case of failure to resume (eg: dead socket).
* It will need to operate under the proxy's lock. The caller is
* responsible for indicating in lpx whether the proxy locks is
* already held (non-zero) or not (zero) so that the function picks it.
*/
int resume_listener(struct listener *l);
int resume_listener(struct listener *l, int lpx);
/*
* This function completely stops a listener. It will need to operate under the
* proxy's lock, the protocol's lock, and the listener's lock. The caller is
* responsible for indicating in lpx, lpr, lli whether the respective locks are
* proxy's lock and the protocol's lock. The caller is
* responsible for indicating in lpx, lpr whether the respective locks are
* already held (non-zero) or not (zero) so that the function picks the missing
* ones, in this order.
*/
void stop_listener(struct listener *l, int lpx, int lpr, int lli);
void stop_listener(struct listener *l, int lpx, int lpr);
/* This function adds the specified listener's file descriptor to the polling
* lists if it is in the LI_LISTEN state. The listener enters LI_READY or

View File

@ -332,13 +332,14 @@ void enable_listener(struct listener *listener)
/*
* This function completely stops a listener. It will need to operate under the
* proxy's lock, the protocol's lock, and the listener's lock. The caller is
* responsible for indicating in lpx, lpr, lli whether the respective locks are
* already held (non-zero) or not (zero) so that the function picks the missing
* ones, in this order. The proxy's listeners count is updated and the proxy is
* It will need to operate under the proxy's lock and the protocol's lock.
* The caller is responsible for indicating in lpx, lpr whether the
* respective locks are already held (non-zero) or not (zero) so that the
* function picks the missing ones, in this order.
* The proxy's listeners count is updated and the proxy is
* disabled and woken up after the last one is gone.
*/
void stop_listener(struct listener *l, int lpx, int lpr, int lli)
void stop_listener(struct listener *l, int lpx, int lpr)
{
struct proxy *px = l->bind_conf->frontend;
@ -355,8 +356,7 @@ void stop_listener(struct listener *l, int lpx, int lpr, int lli)
if (!lpr)
HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
if (!lli)
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
if (l->state > LI_INIT) {
do_unbind_listener(l);
@ -367,8 +367,7 @@ void stop_listener(struct listener *l, int lpx, int lpr, int lli)
proxy_cond_disable(px);
}
if (!lli)
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
if (!lpr)
HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
@ -457,12 +456,18 @@ int default_resume_listener(struct listener *l)
* closes upon SHUT_WR and refuses to rebind. So a common validation path
* involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling
* is disabled. It normally returns non-zero, unless an error is reported.
* It will need to operate under the proxy's lock. The caller is
* responsible for indicating in lpx whether the proxy locks is
* already held (non-zero) or not (zero) so that the function picks it.
*/
int pause_listener(struct listener *l)
int pause_listener(struct listener *l, int lpx)
{
struct proxy *px = l->bind_conf->frontend;
int ret = 1;
if (!lpx)
HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
if (l->state <= LI_PAUSED)
@ -481,6 +486,10 @@ int pause_listener(struct listener *l)
}
end:
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
if (!lpx)
HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);
return ret;
}
@ -493,13 +502,19 @@ int pause_listener(struct listener *l)
* state, it's totally rebound. This can happen if a pause() has completely
* stopped it. If the resume fails, 0 is returned and an error might be
* displayed.
* It will need to operate under the proxy's lock. The caller is
* responsible for indicating in lpx whether the proxy locks is
* already held (non-zero) or not (zero) so that the function picks it.
*/
int resume_listener(struct listener *l)
int resume_listener(struct listener *l, int lpx)
{
struct proxy *px = l->bind_conf->frontend;
int was_paused = px && px->li_paused;
int ret = 1;
if (!lpx)
HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
/* check that another thread didn't to the job in parallel (e.g. at the
@ -530,6 +545,10 @@ int resume_listener(struct listener *l)
}
end:
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
if (!lpx)
HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);
return ret;
}
@ -572,7 +591,7 @@ void dequeue_all_listeners()
/* This cannot fail because the listeners are by definition in
* the LI_LIMITED state.
*/
resume_listener(listener);
resume_listener(listener, 0);
}
}
@ -585,7 +604,7 @@ void dequeue_proxy_listeners(struct proxy *px)
/* This cannot fail because the listeners are by definition in
* the LI_LIMITED state.
*/
resume_listener(listener);
resume_listener(listener, 0);
}
}
@ -1154,7 +1173,7 @@ void listener_accept(struct listener *l)
(!tick_isset(global_listener_queue_task->expire) ||
tick_is_expired(global_listener_queue_task->expire, now_ms))))) {
/* at least one thread has to this when quitting */
resume_listener(l);
resume_listener(l, 0);
/* Dequeues all of the listeners waiting for a resource */
dequeue_all_listeners();
@ -1173,7 +1192,7 @@ void listener_accept(struct listener *l)
* Let's put it to pause in this case.
*/
if (l->rx.proto && l->rx.proto->rx_listening(&l->rx) == 0) {
pause_listener(l);
pause_listener(l, 0);
goto end;
}
@ -1211,7 +1230,7 @@ void listener_release(struct listener *l)
_HA_ATOMIC_DEC(&l->thr_conn[tid]);
if (l->state == LI_FULL || l->state == LI_LIMITED)
resume_listener(l);
resume_listener(l, 0);
/* Dequeues all of the listeners waiting for a resource */
dequeue_all_listeners();

View File

@ -160,7 +160,7 @@ void protocol_stop_now(void)
HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
list_for_each_entry(proto, &protocols, list) {
list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list)
stop_listener(listener, 0, 1, 0);
stop_listener(listener, 0, 1);
}
HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
}
@ -180,7 +180,7 @@ int protocol_pause_all(void)
HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
list_for_each_entry(proto, &protocols, list) {
list_for_each_entry(listener, &proto->receivers, rx.proto_list)
if (!pause_listener(listener))
if (!pause_listener(listener, 0))
err |= ERR_FATAL;
}
HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
@ -202,7 +202,7 @@ int protocol_resume_all(void)
HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
list_for_each_entry(proto, &protocols, list) {
list_for_each_entry(listener, &proto->receivers, rx.proto_list)
if (!resume_listener(listener))
if (!resume_listener(listener, 0))
err |= ERR_FATAL;
}
HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);

View File

@ -2260,7 +2260,7 @@ int pause_proxy(struct proxy *p)
goto end;
list_for_each_entry(l, &p->conf.listeners, by_fe)
pause_listener(l);
pause_listener(l, 1);
if (p->li_ready) {
ha_warning("%s %s failed to enter pause mode.\n", proxy_cap_str(p->cap), p->id);
@ -2289,7 +2289,7 @@ void stop_proxy(struct proxy *p)
HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
list_for_each_entry(l, &p->conf.listeners, by_fe)
stop_listener(l, 1, 0, 0);
stop_listener(l, 1, 0);
if (!(p->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) && !p->li_ready) {
/* might be just a backend */
@ -2318,7 +2318,7 @@ int resume_proxy(struct proxy *p)
fail = 0;
list_for_each_entry(l, &p->conf.listeners, by_fe) {
if (!resume_listener(l)) {
if (!resume_listener(l, 1)) {
int port;
port = get_host_port(&l->rx.addr);
@ -3007,7 +3007,7 @@ static int cli_parse_set_maxconn_frontend(char **args, char *payload, struct app
px->maxconn = v;
list_for_each_entry(l, &px->conf.listeners, by_fe) {
if (l->state == LI_FULL)
resume_listener(l);
resume_listener(l, 1);
}
if (px->maxconn > px->feconn)