BUG/MINOR: listeners: fix suspend/resume of inherited FDs

FDs inherited from a parent process do not deal well with suspend/resume
since commit 59b5da487 ("BUG/MEDIUM: listener: never suspend inherited
sockets") introduced in 2.3. The problem is that we now report that they
cannot be suspended at all, and they return a failure. As such, if a new
process fails to bind and sends SIGTTOU to the previous process, that
one will notice the failure and instantly switch to soft-stop, leaving
no chance to the new process to give up later and signal its failure.

What we need to do, however, is to stop receiving new connections from
such inherited FDs, which just means that the FD must be unsubscribed
from the poller (and resubscribed later if finally it has to stay).
With this a new process can start on the already bound FD without
problem thanks to the absence of polling, and when the old process
stops the new process will be alone on it.

This may be backported as far as 2.4.

diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index 2dbfb64f895e1526dd7187c5a65fab3ee456608b..0c86d6eb6e4257b716839e2dde3a23d82325ed7b 100644 (file)
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -760,22 +760,24 @@ static void tcp_disable_listener(struct listener *l)
 }
 
 /* Suspend a receiver. Returns < 0 in case of failure, 0 if the receiver
- * was totally stopped, or > 0 if correctly suspended.
+ * was totally stopped, or > 0 if correctly suspended. Note that inherited FDs
+ * are neither suspended nor resumed, we only enable/disable polling on them.
  */
 static int tcp_suspend_receiver(struct receiver *rx)
 {
        const struct sockaddr sa = { .sa_family = AF_UNSPEC };
        int ret;
 
-       /* we never do that with a shared FD otherwise we'd break it in the
+       /* We never disconnect a shared FD otherwise we'd break it in the
         * parent process and any possible subsequent worker inheriting it.
+        * Thus we just stop receiving from it.
         */
        if (rx->flags & RX_F_INHERITED)
-               return -1;
+               goto done;
 
        if (connect(rx->fd, &sa, sizeof(sa)) < 0)
                goto check_already_done;
-
+ done:
        fd_stop_recv(rx->fd);
        return 1;
 
@@ -796,7 +798,8 @@ static int tcp_suspend_receiver(struct receiver *rx)
 }
 
 /* Resume a receiver. Returns < 0 in case of failure, 0 if the receiver
- * was totally stopped, or > 0 if correctly suspended.
+ * was totally stopped, or > 0 if correctly resumed. Note that inherited FDs
+ * are neither suspended nor resumed, we only enable/disable polling on them.
  */
 static int tcp_resume_receiver(struct receiver *rx)
 {
@@ -805,7 +808,7 @@ static int tcp_resume_receiver(struct receiver *rx)
        if (rx->fd < 0)
                return 0;
 
-       if (listen(rx->fd, listener_backlog(l)) == 0) {
+       if ((rx->flags & RX_F_INHERITED) || listen(rx->fd, listener_backlog(l)) == 0) {
                fd_want_recv(l->rx.fd);
                return 1;
        }

diff --git a/src/proto_udp.c b/src/proto_udp.c
index 542d287c01e414aa43e0eacfe8af5380a5632b60..1fd92c7ce793bd952880a4b3e16a3324e4c2d797 100644 (file)
--- a/src/proto_udp.c
+++ b/src/proto_udp.c
@@ -176,7 +176,9 @@ static void udp_disable_listener(struct listener *l)
  * suspend the receiver, we want it to stop receiving traffic, which means that
  * the socket must be unhashed from the kernel's socket table. The simple way
  * to do this is to connect to any address that is reachable and will not be
- * used by regular traffic, and a great one is reconnecting to self.
+ * used by regular traffic, and a great one is reconnecting to self. Note that
+ * inherited FDs are neither suspended nor resumed, we only enable/disable
+ * polling on them.
  */
 int udp_suspend_receiver(struct receiver *rx)
 {
@@ -190,14 +192,14 @@ int udp_suspend_receiver(struct receiver *rx)
         * parent process and any possible subsequent worker inheriting it.
         */
        if (rx->flags & RX_F_INHERITED)
-               return -1;
+               goto done;
 
        if (getsockname(rx->fd, (struct sockaddr *)&ss, &len) < 0)
                return -1;
 
        if (connect(rx->fd, (struct sockaddr *)&ss, len) < 0)
                return -1;
-
+ done:
        /* not necessary but may make debugging clearer */
        fd_stop_recv(rx->fd);
        return 1;
@@ -207,7 +209,8 @@ int udp_suspend_receiver(struct receiver *rx)
  * was totally stopped, or > 0 if correctly suspended.
  * The principle is to reverse the change above, we'll break the connection by
  * connecting to AF_UNSPEC. The association breaks and the socket starts to
- * receive from everywhere again.
+ * receive from everywhere again. Note that inherited FDs are neither suspended
+ * nor resumed, we only enable/disable polling on them.
  */
 int udp_resume_receiver(struct receiver *rx)
 {
@@ -216,7 +219,7 @@ int udp_resume_receiver(struct receiver *rx)
        if (rx->fd < 0)
                return 0;
 
-       if (connect(rx->fd, &sa, sizeof(sa)) < 0)
+       if (!(rx->flags & RX_F_INHERITED) && connect(rx->fd, &sa, sizeof(sa)) < 0)
                return -1;
 
        fd_want_recv(rx->fd);