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 1b918b19aca093fab65baaf8b10eb533318c72a7..e5f8236794efdfb9bc12299da1fc7221bf90f773 100644 (file)
--- a/include/haproxy/listener.h
+++ b/include/haproxy/listener.h
@@ -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

diff --git a/src/listener.c b/src/listener.c
index 6f8d4ad6206eaf27a40b3dc9ae7c82b447a565ea..98d384cda91fd9d67e4ce6c21cb4145935a6a60a 100644 (file)
--- a/src/listener.c
+++ b/src/listener.c
@@ -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);
        }
 }
 
@@ -1155,7 +1174,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();
@@ -1174,7 +1193,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;
        }
 
@@ -1212,7 +1231,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();

diff --git a/src/protocol.c b/src/protocol.c
index 7da0727386460e066e7e323a3be6e8a3a4b64fb4..03f70859133ffff5b646f8bab9ba4c514e3d570c 100644 (file)
--- a/src/protocol.c
+++ b/src/protocol.c
@@ -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);

diff --git a/src/proxy.c b/src/proxy.c
index 7b1c96eb434b58a309e03a7b4c1287b591f660b1..7d5989a6ca8381935616c1e5daa2b85b389476de 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -2265,7 +2265,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);
@@ -2294,7 +2294,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 */
@@ -2323,7 +2323,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);
@@ -3012,7 +3012,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)