BUG/MEDIIM: stconn: Flush output data before forwarding close to write side
In process_stream(), we wait to have an empty output channel to forward a
close to the write side (a shutw). However, at the stream-connector level,
when a close is detected on one side and we don't want to keep half-close
connections, the shutw is unconditionally forwarded to the write side. This
typically happens on server side.
At first glance, this bug may truncate messages. But depending on the muxes
and the stream states, the bug may be more visible. On recent versions
(2.8-dev and 2.7) and on 2.2 and 2.0, the stream may be freezed, waiting for
the client timeout, if the client mux is unable to forward data because the
client is too slow _AND_ the response channel is not empty _AND_ the server
closes its connection _AND_ the server mux has forwarded all data to the
upper layer _AND_ the client decides to send some data and to close its
connection. On 2.6 and 2.4, it is worst. Instead of a freeze, the client mux
is woken up in loop.
Of course, conditions are pretty hard to meet. Especially because it is highly
time dependent. For what it's worth, I reproduce it with tcploop on client and
server sides and a basic HTTP configuration for HAProxy:
* client: tcploop -v 8889 C S:"GET / HTTP/1.1\r\nConnection: upgrade\r\n\r\n" P5000 S:"1234567890" K
* server: tcploop -v 8000 L A R S:"HTTP/1.1 101 ok\r\nConnection: upgrade\r\n\r\n" P2000 S2660000 F R
On 2.8-dev, without this patch, the stream is freezed and when the client
connection timed out, client data are truncated and '--cL' is reported in
logs. With the patch, the client data are forwarded to the server and the
connection is closed. A '--CD' is reported in logs.
It is an old bug. It was probably introduced with the multiplexers. To fix
it, in stconn (Formerly the stream-interface), we must wait all output data
be flushed before forwarding close to write side.
This patch must be backported as far as 2.2 and must be evaluated for 2.0.
diff --git a/src/stconn.c b/src/stconn.c
index 44f591c..178155e 100644
--- a/src/stconn.c
+++ b/src/stconn.c
@@ -518,7 +518,7 @@
if (sc->flags & SC_FL_ISBACK)
__sc_strm(sc)->conn_exp = TICK_ETERNITY;
}
- else if (sc->flags & SC_FL_NOHALF) {
+ else if ((sc->flags & SC_FL_NOHALF) && channel_is_empty(ic)) {
/* we want to immediately forward this close to the write side */
return sc_app_shutw(sc);
}
@@ -662,7 +662,7 @@
if (sc->flags & SC_FL_ISBACK)
__sc_strm(sc)->conn_exp = TICK_ETERNITY;
}
- else if (sc->flags & SC_FL_NOHALF) {
+ else if ((sc->flags & SC_FL_NOHALF) && channel_is_empty(ic)) {
/* we want to immediately forward this close to the write side */
return sc_app_shutw_conn(sc);
}
@@ -890,7 +890,7 @@
if (sc->flags & SC_FL_ISBACK)
__sc_strm(sc)->conn_exp = TICK_ETERNITY;
}
- else if (sc->flags & SC_FL_NOHALF) {
+ else if ((sc->flags & SC_FL_NOHALF) && channel_is_empty(ic)) {
/* we want to immediately forward this close to the write side */
return sc_app_shutw_applet(sc);
}
@@ -1249,7 +1249,7 @@
if (oc->flags & CF_SHUTW)
goto do_close;
- if (sc->flags & SC_FL_NOHALF) {
+ if ((sc->flags & SC_FL_NOHALF) && channel_is_empty(ic)) {
/* we want to immediately forward this close to the write side */
/* force flag on ssl to keep stream in cache */
sc_conn_shutw(sc, CO_SHW_SILENT);