RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2024-12-25 14:12:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
Willy Tarreau 2017-12-22 18:46:33 +01:00
parent d4569d1937
commit a48c141f44
2 changed files with 9 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
include/proto/connection.h
View File

@ -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);
}

2
src/mux_pt.c
View File

@ -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);
}