Hi Luke, On Tue, Jan 29, 2019 at 10:52:15AM +0000, Luke Seelenbinder wrote: > Hi Willy, > > > By the way, how do you manage to cancel a single stream in the browser ? > > Pressing Esc might break all of them I guess ? Thus I'm uncertain how to > > achieve this. > > So we're in a very specific use-case of delivering map tiles, which are > predominately requested via Leaflet.js. Leaflet.js cancels requests when a > particular map tile is no longer needed (e.g., zoomed/panned past), so > individual request cancellation is a very normal behavior in our setup.
OK so this definitely makes sense. > Here's a demo link that you could find helpful to capture stream information: > https://stadiamaps.com/demo-raster.html. To replicate the issue, I zoom in > twice (hit the plus button twice), which creates a set of tile requests for > zoom X and then zoom Y. All requests for X are canceled when the Y set is > sent. In h2 backend mode, this means most of Y fails, because the X set broke > all the currently open connections/streams that Y tries to reuse. In h1.1 > backend mode, it means a small subset of Y may fail, because HAProxy > improperly closes the client connection during X. Thank you for this. I finally managed to track it down to something very stupid : we are supposed to cause a connection error when seeing an attempt to create a new stream with an already existing ID, which translates to seeing a HEADERS frame on a closed stream in practice. This was true on the frontend side, but not on the backend, because when a client aborts a stream, it immediately turns to the closed state (always the same problem) and if the server happens to respond on this stream, it will start with a HEADERS frame with an ID of a closed stream, hence we kill the connection. So I've added a test for the side to this check, because obviously HEADERS frames only create streams from clients to servers, not the other way around. I've merged the attached patch that fixes the problem for me, please try it, it should apply cleanly on top of 1.9.3. Thanks, Willy
>From 3ad5d31bdf66c3a9449bb4af4cb131ff8e2ca662 Mon Sep 17 00:00:00 2001 From: Willy Tarreau <[email protected]> Date: Tue, 29 Jan 2019 18:33:26 +0100 Subject: BUG/MEDIUM: mux-h2: only close connection on request frames on closed streams A subtle bug was introduced with H2 on the backend. RFC7540 states that an attempt to create a stream on an ID not higher than the max known is a connection error. This was translated into rejecting HEADERS frames for closed streams. But with H2 on the backend, if the client aborts and causes an RST_STREAM to be emitted, the stream is effectively closed, and if/once the server responds, it starts by emitting a HEADERS frame with this ID thus it is interpreted as a connection error. This test must of course consider the side the mux is installed on and not take this for a connection error on responses. The effect is that an aborted stream on an outgoing H2 connection, for example due to a client stopping a transfer with option abortonclose set, would lead to an abort of all other streams. In the logs, this appears as one or several CD-- line(s) followed by one or several SD-- lines which are victims. Thanks to Luke Seelenbinder for reporting this problem and providing enough elements to help understanding how to reproduce it. This fix must be backported to 1.9. --- src/mux_h2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mux_h2.c b/src/mux_h2.c index 90895e288..f74ae3288 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -2344,7 +2344,7 @@ static void h2_process_demux(struct h2c *h2c) * Some frames have to be silently ignored as well. */ if (h2s->st == H2_SS_CLOSED && h2c->dsi) { - if (h2_ft_bit(h2c->dft) & H2_FT_HDR_MASK) { + if (!(h2c->flags & H2_CF_IS_BACK) && h2_ft_bit(h2c->dft) & H2_FT_HDR_MASK) { /* #5.1.1: The identifier of a newly * established stream MUST be numerically * greater than all streams that the initiating -- 2.20.1
