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 f1513a81f5a40a7af5868f5b600dd2acd550dd83..9503d12f65cacf530c09d4739b23f98f7332288e 100644 (file)
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -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));
 }
 
 /*