From: Willy Tarreau <w@1wt.eu>
Date: Thu, 8 Oct 2020 13:36:46 +0000 (+0200)
Subject: BROKEN/MEDIUM: listeners: rework the unbind logic to make it idempotent
X-Git-Tag: v2.3-dev6~12
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=87acd4e84815b6d7116ca28f45c8545eb7026169;p=thirdparty%2Fhaproxy.git

BROKEN/MEDIUM: listeners: rework the unbind logic to make it idempotent

BROKEN: the failure rate on reg-tests/seamless-reload/abns_socket.vtc has
significantly increased for no obvious reason. It fails 99% of the time vs
10% before.

do_unbind_listener() is not logical and is not even idempotent. It must
not touch the fd if already -1, which also means not touch the receiver.
In addition, when performing a partial stop on a socket (not closing),
we know the socket remains in the listening state yet it's marked as
LI_ASSIGNED, which is confusing as it doesn't translate its real state.

With this change, we make sure that FDs marked for close end up in
ASSIGNED state and that those which are really bound and on which a
listen() was made (i.e. not pause) remain in LISTEN state. This is what
is closest to reality.

Ideally this function should become a default proto->unbind() one but
it may still keep a bit too much state logic to become generalized to
other protocols (e.g. QUIC).
---

diff --git a/src/listener.c b/src/listener.c
index 364d061ac7..92463534b9 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -562,20 +562,32 @@ void dequeue_proxy_listeners(struct proxy *px)
  */
 void do_unbind_listener(struct listener *listener, int do_close)
 {
-	if (listener->state == LI_READY)
-		listener->rx.proto->disable(listener);
-
 	MT_LIST_DEL(&listener->wait_queue);
 
-	if (listener->state >= LI_PAUSED) {
+	if (listener->state <= LI_ASSIGNED)
+		goto out_close;
+
+	if (listener->rx.fd == -1) {
 		listener_set_state(listener, LI_ASSIGNED);
+		goto out_close;
+	}
+
+	if (listener->state >= LI_PAUSED) {
+		if (listener->state >= LI_READY) {
+			listener->rx.proto->disable(listener);
+			listener_set_state(listener, LI_LISTEN);
+		}
 		listener->rx.proto->rx_disable(&listener->rx);
 	}
 
-	if (do_close && listener->rx.fd != -1) {
-		fd_delete(listener->rx.fd);
+ out_close:
+	if (do_close) {
 		listener->rx.flags &= ~RX_F_BOUND;
+		if (listener->rx.fd != -1)
+			fd_delete(listener->rx.fd);
 		listener->rx.fd = -1;
+		if (listener->state > LI_ASSIGNED)
+			listener_set_state(listener, LI_ASSIGNED);
 	}
 }