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 364d061ac7b76f445dc7e5deeefaac2c709648f3..92463534b9c48c3b09ded9833f68e142b55e69c9 100644 (file)
--- 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);
        }
 }