]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: muxes: Report the Last read with a dedicated flag
authorChristopher Faulet <cfaulet@haproxy.com>
Fri, 8 Mar 2019 08:23:46 +0000 (09:23 +0100)
committerWilly Tarreau <w@1wt.eu>
Mon, 18 Mar 2019 14:50:23 +0000 (15:50 +0100)
For conveniance, in HTTP muxes (h1 and h2), the end of the stream and the end of
the message are reported the same way to the stream, by setting the flag
CS_FL_EOS. In the stream-interface, when CS_FL_EOS is detected, a shutdown for
read is reported on the channel side. This is historical. With the legacy HTTP
layer, because the parsing is done by the stream in HTTP analyzers, the EOS
really means a shutdown for read.

Most of time, for muxes h1 and h2, it works pretty well, especially because the
keep-alive is handled by the muxes. The stream is only used for one
transaction. So mixing EOS and EOM is good enough. But not everytime. For now,
client aborts are only reported if it happens before the end of the request. It
is an error and it is properly handled. But because the EOS was already
reported, client aborts after the end of the request are silently
ignored. Eventually an error can be reported when the response is sent to the
client, if the sending fails. Otherwise, if the server does not reply fast
enough, an error is reported when the server timeout is reached. It is the
expected behaviour, excpect when the option abortonclose is set. In this case,
we must report an error when the client aborts. But as said before, this event
can be ignored. So to be short, for now, the abortonclose is broken.

In fact, it is a design problem and we have to rethink all channel's flags and
probably the conn-stream ones too. It is important to split EOS and EOM to not
loose information anymore. But it is not a small job and the refactoring will be
far from straightforward.

So for now, temporary flags are introduced. When the last read is received, the
flag CS_FL_READ_NULL is set on the conn-stream. This way, we can set the flag
SI_FL_READ_NULL on the stream interface. Both flags are persistant. And to be
sure to wake the stream, the event CF_READ_NULL is reported. So the stream will
always have the chance to handle the last read.

This patch must be backported to 1.9 because it will be used by another patch to
fix the option abortonclose.

include/types/connection.h
include/types/stream_interface.h
src/mux_h1.c
src/mux_h2.c
src/mux_pt.c
src/stream_interface.c

index 96728b35f5ecd0f45c7554a526f42756568987cc..33935bbfcc22689fc92d38c4a2e4df25808d4f0b 100644 (file)
@@ -87,6 +87,7 @@ enum {
        CS_FL_ERR_PENDING   = 0x00000800,  /* An error is pending, but there's still data to be read */
        CS_FL_EOS           = 0x00001000,  /* End of stream delivered to data layer */
        CS_FL_REOS          = 0x00002000,  /* End of stream received (buffer not empty) */
+       CS_FL_READ_NULL     = 0x00004000,  /* Last read received */
        CS_FL_WAIT_FOR_HS   = 0x00010000,  /* This stream is waiting for handhskae */
        CS_FL_KILL_CONN     = 0x00020000,  /* must kill the connection when the CS closes */
 
index 8be3ee98bd277612e293b2d228fc2284a57ad193..d1b81354bb1644b4faab4e1f8caa1e3a6419402d 100644 (file)
@@ -72,7 +72,7 @@ enum {
        SI_FL_NOLINGER   = 0x00000080,  /* may close without lingering. One-shot. */
        SI_FL_NOHALF     = 0x00000100,  /* no half close, close both sides at once */
        SI_FL_SRC_ADDR   = 0x00001000,  /* get the source ip/port with getsockname */
-       /* unused:         0x00002000 */
+       SI_FL_READ_NULL  = 0x00000200,   /* Last read reported */
        SI_FL_WANT_GET   = 0x00004000,  /* a stream-int would like to get some data from the buffer */
        SI_FL_CLEAN_ABRT = 0x00008000,  /* SI_FL_ERR is used to report aborts, and not SHUTR */
 
index c09d008f406bc6c6d3de85b30f916d461d92d11c..8b686fe25f93ef3f107fa5ff009df4309ee68b86 100644 (file)
@@ -1777,7 +1777,7 @@ static int h1_recv(struct h1c *h1c)
                h1_wake_stream_for_recv(h1s);
 
        if (conn_xprt_read0_pending(conn) && h1s && h1s->cs)
-               h1s->cs->flags |= CS_FL_REOS;
+               h1s->cs->flags |= (CS_FL_REOS|CS_FL_READ_NULL);
        if (!b_data(&h1c->ibuf))
                h1_release_buf(h1c, &h1c->ibuf);
        else if (!buf_room_for_htx_data(&h1c->ibuf))
@@ -1885,7 +1885,7 @@ static int h1_process(struct h1c * h1c)
                if (h1c->flags & H1C_F_CS_ERROR || conn->flags & CO_FL_ERROR)
                        flags |= CS_FL_ERROR;
                if (conn_xprt_read0_pending(conn))
-                       flags |= CS_FL_EOS;
+                       flags |= (CS_FL_REOS|CS_FL_READ_NULL);
                h1s->cs->flags |= flags;
                h1s->cs->data_cb->wake(h1s->cs);
        }
index af69304f45224aba3234208eaa5c5af8fea50c5d..f54b67d2021bd4bc1f8e20e1e20863fc63c8dfbf 100644 (file)
@@ -1423,7 +1423,7 @@ static void h2_wake_some_streams(struct h2c *h2c, int last, uint32_t flags)
                flags |= CS_FL_ERR_PENDING;
 
        if (conn_xprt_read0_pending(h2c->conn))
-               flags |= CS_FL_REOS;
+               flags |= (CS_FL_REOS|CS_FL_READ_NULL);
 
        /* Wake all streams with ID > last */
        node = eb32_lookup_ge(&h2c->streams_by_id, last + 1);
index 4f53920c418579de034ecc69289c411b195d2af0..74c802ea82d4cb85ef5690b9ec9cba4ad10fcc87 100644 (file)
@@ -255,7 +255,7 @@ static size_t mux_pt_rcv_buf(struct conn_stream *cs, struct buffer *buf, size_t
        if (conn_xprt_read0_pending(cs->conn)) {
                if (ret == 0)
                        cs->flags &= ~(CS_FL_RCV_MORE | CS_FL_WANT_ROOM);
-               cs->flags |= CS_FL_EOS;
+               cs->flags |= (CS_FL_EOS|CS_FL_READ_NULL);
        }
        if (cs->conn->flags & CO_FL_ERROR) {
                if (ret == 0)
@@ -298,7 +298,7 @@ static int mux_pt_rcv_pipe(struct conn_stream *cs, struct pipe *pipe, unsigned i
 
        ret = cs->conn->xprt->rcv_pipe(cs->conn, pipe, count);
        if (conn_xprt_read0_pending(cs->conn))
-               cs->flags |= CS_FL_EOS;
+               cs->flags |= (CS_FL_EOS|CS_FL_READ_NULL);
        if (cs->conn->flags & CO_FL_ERROR)
                cs->flags |= CS_FL_ERROR;
        return (ret);
index e9fe00f6dc440adadffd00c2ec121ce53b88e883..8fba981876e679646e6210bca8237b4d8753682a 100644 (file)
@@ -594,6 +594,17 @@ static int si_cs_process(struct conn_stream *cs)
                oc->flags |= CF_WRITE_NULL;
        }
 
+       /* The last read was received but not reported to the channel
+        * (CS_FL_READ_NULL set but not SI_FL_READ_NULL). So do it, even if the
+        * EOS was already reported.
+        *
+        * NOTE: It is a temporary fix to handle client aborts.
+        */
+       if (cs->flags & CS_FL_READ_NULL && !(si->flags & SI_FL_READ_NULL)) {
+               si->flags |= SI_FL_READ_NULL;
+               ic->flags |= CF_READ_NULL;
+       }
+
        /* Second step : update the stream-int and channels, try to forward any
         * pending data, then possibly wake the stream up based on the new
         * stream-int status.