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
diff --git a/include/haproxy/listener.h b/include/haproxy/listener.h
index 1b918b1..e5f8236 100644
--- a/include/haproxy/listener.h
+++ b/include/haproxy/listener.h
@@ -42,23 +42,29 @@
* 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
diff --git a/src/listener.c b/src/listener.c
index 6f8d4ad..98d384c 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -332,13 +332,14 @@
/*
* 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 @@
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 @@
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 @@
* 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 @@
}
end:
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
+
+ if (!lpx)
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);
+
return ret;
}
@@ -493,13 +502,19 @@
* 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 @@
}
end:
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
+
+ if (!lpx)
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);
+
return ret;
}
@@ -572,7 +591,7 @@
/* 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 @@
/* This cannot fail because the listeners are by definition in
* the LI_LIMITED state.
*/
- resume_listener(listener);
+ resume_listener(listener, 0);
}
}
@@ -1155,7 +1174,7 @@
(!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();
@@ -1174,7 +1193,7 @@
* 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;
}
@@ -1212,7 +1231,7 @@
_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();
diff --git a/src/protocol.c b/src/protocol.c
index 7da0727..03f7085 100644
--- a/src/protocol.c
+++ b/src/protocol.c
@@ -160,7 +160,7 @@
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 @@
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 @@
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);
diff --git a/src/proxy.c b/src/proxy.c
index 7b1c96e..7d5989a 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -2265,7 +2265,7 @@
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);
@@ -2294,7 +2294,7 @@
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 */
@@ -2323,7 +2323,7 @@
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);
@@ -3012,7 +3012,7 @@
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)