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 001328873c352e5e4b1df0dcc8facaf2fc1408aa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 614f99ee0a9f71f85f12e88b0a113ede1e51ae40)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit e3b32c01df854610096487c1f1430224892248a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/include/haproxy/listener.h b/include/haproxy/listener.h
index 3fb078f..bb502c6 100644
--- a/include/haproxy/listener.h
+++ b/include/haproxy/listener.h
@@ -39,23 +39,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 0ffd0fe..6334974 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -297,13 +297,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;
@@ -320,8 +321,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);
@@ -332,8 +332,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);
@@ -422,12 +421,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 ((global.mode & (MODE_DAEMON | MODE_MWORKER)) &&
@@ -450,6 +455,10 @@
}
end:
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
+
+ if (!lpx)
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);
+
return ret;
}
@@ -462,13 +471,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
@@ -503,6 +518,10 @@
}
end:
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
+
+ if (!lpx)
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);
+
return ret;
}
@@ -545,7 +564,7 @@
/* This cannot fail because the listeners are by definition in
* the LI_LIMITED state.
*/
- resume_listener(listener);
+ resume_listener(listener, 0);
}
}
@@ -558,7 +577,7 @@
/* This cannot fail because the listeners are by definition in
* the LI_LIMITED state.
*/
- resume_listener(listener);
+ resume_listener(listener, 0);
}
}
@@ -1084,7 +1103,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();
@@ -1103,7 +1122,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;
}
@@ -1141,7 +1160,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 767e03a..1806bed 100644
--- a/src/protocol.c
+++ b/src/protocol.c
@@ -155,7 +155,7 @@
list_for_each_entry(proto, &protocols, list) {
list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list)
if (!listener->bind_conf->frontend->grace)
- stop_listener(listener, 0, 1, 0);
+ stop_listener(listener, 0, 1);
}
HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
}
@@ -175,7 +175,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);
@@ -197,7 +197,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 5c105ea..7a7faf2 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -2050,7 +2050,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);
@@ -2079,7 +2079,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->disabled && !p->li_ready) {
/* might be just a backend */
@@ -2108,7 +2108,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);
@@ -2811,7 +2811,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)