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.
diff --git a/src/stream_interface.c b/src/stream_interface.c
index f1513a8..9503d12 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -594,6 +594,10 @@
* 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 @@
}
/* 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 @@
/* 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 @@
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 @@
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 @@
}
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));
}
/*