RepoMirrors
/
haproxy
mirror of http://git.haproxy.org/git/haproxy.git/
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

BUG/MEDIUM: stream-int: Don't loose events on the CS when an EOS is reported 

In si_cs_recv(), when a shutdown for reads is handled, the conn-stream may be
closed. It happens when the ouput channel is closed for writes or if
SI_FL_NOHALF is set on the stream-interface. In this case, conn-stream's flags
are reset. Thus, if an error (CS_FL_ERROR) or an end of input (CS_FL_EOI) is
reported by the mux, the event is lost. si_cs_recv() does not report these
events by itself. It relies on si_cs_process() to report them to the
stream-interface and/or the channel.

For instance, if CS_FL_EOS and CS_FL_EOI are set by the H1 multiplexer during a
call to si_cs_recv() on the server side, if the conn-stream is closed (read0 +
SI_FL_NOHALF), the CS_FL_EOI flag is lost. Thus, this may lead the stream to
interpret it as a server abort.

Now, conn-stream's flags are processed at the end of si_cs_recv(). The function
is responsible to set the right flags on the stream-interface and/or the
channel. Due to this patch, the function is now almost linear. Except some early
checks at the beginning, there is only one return statement. It also fixes a
potential bug because of an inconsistency between the splicing and the buffered
receipt. On the first case, CS_FL_EOS if handled before errors on the connection
or the conn-stream. On the second one, it is the opposite.

This patch must be backported to 2.0 and 1.9.
This commit is contained in:
Christopher Faulet 2019-11-20 11:56:33 +01:00
parent 3c8bde88ca
commit 36b536d6c8
1 changed files with 36 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
src/stream_interface.c
View File

@ -594,6 +594,10 @@ static int si_cs_process(struct conn_stream *cs)
* after process_stream() noticed there were an error, and decided
* to retry to connect, the connection may still have CO_FL_ERROR,
* and we don't want to add SI_FL_ERR back
*
* Note: This test is only required because si_cs_process is also the SI
* wake callback. Otherwise si_cs_recv()/si_cs_send() already take
* care of it.
*/
if (si->state >= SI_ST_CON &&
(conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR))
@ -619,7 +623,12 @@ static int si_cs_process(struct conn_stream *cs)
}
/* Report EOI on the channel if it was reached from the mux point of
* view. */
* view.
*
* Note: This test is only required because si_cs_process is also the SI
* wake callback. Otherwise si_cs_recv()/si_cs_send() already take
* care of it.
*/
if ((cs->flags & CS_FL_EOI) && !(ic->flags & CF_EOI))
ic->flags |= (CF_EOI|CF_READ_PARTIAL);
@ -1226,7 +1235,7 @@ int si_cs_recv(struct conn_stream *cs)
/* stop here if we reached the end of data */
if (cs->flags & CS_FL_EOS)
goto out_shutdown_r;
goto end_recv;
/* stop immediately on errors. Note that we DON'T want to stop on
* POLL_ERR, as the poller might report a write error while there
@ -1238,7 +1247,7 @@ int si_cs_recv(struct conn_stream *cs)
if (!conn_xprt_ready(conn))
return 0;
if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
return 1; // We want to make sure si_cs_wake() is called, so that process_strema is woken up, on failure
goto end_recv;
}
/* prepare to detect if the mux needs more room */
@ -1294,11 +1303,8 @@ int si_cs_recv(struct conn_stream *cs)
ic->flags |= CF_READ_PARTIAL;
}
if (cs->flags & CS_FL_EOS)
goto out_shutdown_r;
if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
return 1;
if (conn->flags & CO_FL_ERROR || cs->flags & (CS_FL_EOS|CS_FL_ERROR))
goto end_recv;
if (conn->flags & CO_FL_WAIT_ROOM) {
/* the pipe is full or we have read enough data that it
@ -1468,32 +1474,36 @@ int si_cs_recv(struct conn_stream *cs)
}
end_recv:
if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
return 1;
/* Report EOI on the channel if it was reached from the mux point of
* view. */
if ((cs->flags & CS_FL_EOI) && !(ic->flags & CF_EOI))
ic->flags |= (CF_EOI|CF_READ_PARTIAL);
if (cs->flags & CS_FL_EOS)
if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR) {
cs->flags |= CS_FL_ERROR;
si->flags |= SI_FL_ERR;
}
else if (cs->flags & CS_FL_EOS) {
/* connection closed */
goto out_shutdown_r;
/* Subscribe to receive events if we're blocking on I/O */
if (!si_rx_blocked(si)) {
if (conn->flags & CO_FL_CONNECTED) {
/* we received a shutdown */
ic->flags |= CF_READ_NULL;
if (ic->flags & CF_AUTO_CLOSE)
channel_shutw_now(ic);
stream_int_read0(si);
}
}
else if (!si_rx_blocked(si)) {
/* Subscribe to receive events if we're blocking on I/O */
conn->mux->subscribe(cs, SUB_RETRY_RECV, &si->wait_event);
si_rx_endp_done(si);
} else {
si_rx_endp_more(si);
}
return (cur_read != 0) || si_rx_blocked(si) || (cs->flags & CS_FL_EOI);
out_shutdown_r:
if (conn->flags & CO_FL_CONNECTED) {
/* we received a shutdown */
ic->flags |= CF_READ_NULL;
if (ic->flags & CF_AUTO_CLOSE)
channel_shutw_now(ic);
stream_int_read0(si);
}
return 1;
return (cur_read != 0) ||
si_rx_blocked(si) ||
(cs->flags & (CS_FL_EOI|CS_FL_EOS|CS_FL_ERROR));
}
/*