BUG/MEDIUM: connections: Always call shutdown, with no linger.

Revert commit fe4abe62c7c5206dff1802f42d17014e198b9141.
The goal was to make sure for health-checks, we would not get sockets in
TIME_WAIT. To do so, we would not call shutdown() if linger_risk is set.
However that is wrong, and that means shutw would never be forwarded to
the server, and thus we could get connection that are never properly closed.
Instead, to fix the original problem as described here :
https://www.mail-archive.com/haproxy@formilux.org/msg34080.html
Just make sure the checks code call cs_shutr() before calling cs_shutw().
If shutr has been called, conn_sock_shutw() will make no attempt to call
shutdown(), as it knows close() will be called.
We should really review and revamp the shutr/shutw code, as described in
github issue #142.

This should be backported to 1.9 and 2.0.

diff --git a/include/proto/connection.h b/include/proto/connection.h
index 1e5fc9c9e641a28c70fed3896483efd387e5ec8d..115e7aaa3bc9a03b993b23d8363bc51d6a246387 100644 (file)
--- a/include/proto/connection.h
+++ b/include/proto/connection.h
@@ -352,8 +352,7 @@ static inline void conn_sock_shutw(struct connection *c, int clean)
        /* don't perform a clean shutdown if we're going to reset or
         * if the shutr was already received.
         */
-       if (conn_ctrl_ready(c) && !(c->flags & CO_FL_SOCK_RD_SH) && clean &&
-           !fdtab[c->handle.fd].linger_risk)
+       if (conn_ctrl_ready(c) && !(c->flags & CO_FL_SOCK_RD_SH) && clean)
                shutdown(c->handle.fd, SHUT_WR);
 }
 

diff --git a/src/checks.c b/src/checks.c
index e31eb1735935d3eaf7c5af95b3efe93b22435b91..68da2a3766f641d7e35ad564f20155bad802d236 100644 (file)
--- a/src/checks.c
+++ b/src/checks.c
@@ -1382,6 +1382,11 @@ static void __event_srv_chk_r(struct conn_stream *cs)
         * range quickly.  To avoid sending RSTs all the time, we first try to
         * drain pending data.
         */
+       /* Call cs_shutr() first, to add the CO_FL_SOCK_RD_SH flag on the
+        * connection, to make sure cs_shutw() will not lead to a shutdown()
+        * that would provoke TIME_WAITs.
+        */
+       cs_shutr(cs, CS_SHR_DRAIN);
        cs_shutw(cs, CS_SHW_NORMAL);
 
        /* OK, let's not stay here forever */