BUG/MAJOR: connection: refine the situations where we don't send shutw()

Since commit f9ce57e ("MEDIUM: connection: make conn_sock_shutw() aware
of lingering"), we refrain from performing the shutw() on the socket if
there is no lingering risk. But there is a problem with this in tunnel
and in TCP modes where a client is explicitly allowed to send a shutw
to the server, eventhough it it risky.

Not doing it creates this situation reported by Ricardo Fraile and
diagnosed by Christopher : a typical HTTP client (eg: curl) connecting
via the config below to an HTTP server would receive its response,
immediately close while the server remains in keep-alive mode. The
shutr() received by haproxy from the client is "propagated" to the
server side but not acted upon because fdtab[fd].linger_risk is set,
so we expect that the next close will immediately complete this
operation.

  listen proxy-tcp
    bind 127.0.0.1:8888
    mode tcp
    timeout connect 5s
    timeout server  10s
    timeout client  10s
    server server1 127.0.0.1:8000

But since the whole stream will not end until the server closes in
turn, the server doesn't close and haproxy expires on server timeout.
This problem has already struck by waking up an older bug and was
partially fixed with commit 8059351 ("BUG/MEDIUM: http: don't disable
lingering on requests with tunnelled responses") though it was not
enough.

The problem is that linger_risk is not suited here. In fact we need to
know whether or not it is desired to close normally or silently, and
whether or not a shutr() has already been received on this connection.

This is the approach this patch takes, and it solves the problem for
the various difficult modes (tcp, http-server-close, pretend-keepalive).

This fix needs to be backported to 1.8. Many thanks to Ricardo for
providing very detailed traces and configurations.

diff --git a/include/proto/connection.h b/include/proto/connection.h
index 0efce993b2e2ac3a1bbd3a06e54f51c09c22b4cd..64117641c505e8a46ca3704a61b1905cc4837fcb 100644 (file)
--- a/include/proto/connection.h
+++ b/include/proto/connection.h
@@ -505,17 +505,21 @@ static inline void conn_sock_read0(struct connection *c)
 }
 
 /* write shutdown, indication that the upper layer is not willing to send
- * anything anymore and wants to close after pending data are sent.
+ * anything anymore and wants to close after pending data are sent. The
+ * <clean> argument will allow not to perform the socket layer shutdown if
+ * equal to 0.
  */
-static inline void conn_sock_shutw(struct connection *c)
+static inline void conn_sock_shutw(struct connection *c, int clean)
 {
        c->flags |= CO_FL_SOCK_WR_SH;
        conn_refresh_polling_flags(c);
        __conn_sock_stop_send(c);
        conn_cond_update_sock_polling(c);
 
-       /* don't perform a clean shutdown if we're going to reset */
-       if (conn_ctrl_ready(c) && !fdtab[c->handle.fd].linger_risk)
+       /* 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)
                shutdown(c->handle.fd, SHUT_WR);
 }
 

diff --git a/src/mux_pt.c b/src/mux_pt.c
index b32fa8edc93736a59cdc088e641b9fe53c696db5..a68b962154e6159d785bf5143d33c80a32ec71ff 100644 (file)
--- a/src/mux_pt.c
+++ b/src/mux_pt.c
@@ -151,7 +151,7 @@ static void mux_pt_shutw(struct conn_stream *cs, enum cs_shw_mode mode)
        if (conn_xprt_ready(cs->conn) && cs->conn->xprt->shutw)
                cs->conn->xprt->shutw(cs->conn, (mode == CS_SHW_NORMAL));
        if (!(cs->flags & CS_FL_SHR))
-               conn_sock_shutw(cs->conn);
+               conn_sock_shutw(cs->conn, (mode == CS_SHW_NORMAL));
        else
                conn_full_close(cs->conn);
 }