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

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
+ * proxy's lock and the protocol's lock. The caller is
- * responsible for indicating in lpx, lpr, lli whether the respective locks are
+ * 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

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
+ * It will need to operate under the proxy's lock and the protocol's lock.
- * responsible for indicating in lpx, lpr, lli whether the respective locks are
+ * The caller is responsible for indicating in lpx, lpr whether the
- * already held (non-zero) or not (zero) so that the function picks the missing
+ * respective locks are already held (non-zero) or not (zero) so that the
- * ones, in this order. The proxy's listeners count is updated and the proxy is
+ * 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();

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);

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)