BUG/MEDIUM: h2: always add a stream to the send or fctl list when blocked
When a stream blocks on a mux buffer full/unallocated or on connection
flow control, a flag among H2_SF_MUX_M* is set, but the stream is not
always added to the connection's list. It's properly done when the
operations are performed from the connection handler but not always when
done from the stream handler. For instance, a simple shutr or shutw may
fail by lack of room. If it's immediately followed by a call to h2_detach(),
the stream remains lying around in no list at all, and prevents the
connection from ending. This problem is actually quite difficult to
trigger and seems to require some large objects and low server-side
timeouts.
This patch covers all identified paths. Some are redundant but since the
code will change and will be simplified in 1.9, it's better to stay on
the safe side here for now. It must be backported to 1.8.
diff --git a/src/mux_h2.c b/src/mux_h2.c
index 264b207..a22ea2a 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2505,17 +2505,25 @@
*/
if (!(h2s->flags & H2_SF_RST_SENT) &&
h2s_send_rst_stream(h2s->h2c, h2s) <= 0)
- return;
+ goto add_to_list;
if (!(h2s->flags & H2_SF_OUTGOING_DATA) &&
!(h2s->h2c->flags & (H2_CF_GOAWAY_SENT|H2_CF_GOAWAY_FAILED)) &&
h2c_send_goaway_error(h2s->h2c, h2s) <= 0)
- return;
+ goto add_to_list;
if (h2s->h2c->mbuf->o && !(cs->conn->flags & CO_FL_XPRT_WR_ENA))
conn_xprt_want_send(cs->conn);
h2s_close(h2s);
+
+ add_to_list:
+ if (LIST_ISEMPTY(&h2s->list)) {
+ if (h2s->flags & H2_SF_BLK_MFCTL)
+ LIST_ADDQ(&h2s->h2c->fctl_list, &h2s->list);
+ else if (h2s->flags & (H2_SF_BLK_MBUSY|H2_SF_BLK_MROOM))
+ LIST_ADDQ(&h2s->h2c->send_list, &h2s->list);
+ }
}
static void h2_shutw(struct conn_stream *cs, enum cs_shw_mode mode)
@@ -2530,7 +2538,7 @@
if (!(h2s->flags & (H2_SF_ES_SENT|H2_SF_RST_SENT)) &&
h2_send_empty_data_es(h2s) <= 0)
- return;
+ goto add_to_list;
if (h2s->st == H2_SS_HREM)
h2s_close(h2s);
@@ -2544,18 +2552,26 @@
*/
if (!(h2s->flags & H2_SF_RST_SENT) &&
h2s_send_rst_stream(h2s->h2c, h2s) <= 0)
- return;
+ goto add_to_list;
if (!(h2s->flags & H2_SF_OUTGOING_DATA) &&
!(h2s->h2c->flags & (H2_CF_GOAWAY_SENT|H2_CF_GOAWAY_FAILED)) &&
h2c_send_goaway_error(h2s->h2c, h2s) <= 0)
- return;
+ goto add_to_list;
h2s_close(h2s);
}
if (h2s->h2c->mbuf->o && !(cs->conn->flags & CO_FL_XPRT_WR_ENA))
conn_xprt_want_send(cs->conn);
+
+ add_to_list:
+ if (LIST_ISEMPTY(&h2s->list)) {
+ if (h2s->flags & H2_SF_BLK_MFCTL)
+ LIST_ADDQ(&h2s->h2c->fctl_list, &h2s->list);
+ else if (h2s->flags & (H2_SF_BLK_MBUSY|H2_SF_BLK_MROOM))
+ LIST_ADDQ(&h2s->h2c->send_list, &h2s->list);
+ }
}
/* Decode the payload of a HEADERS frame and produce the equivalent HTTP/1
@@ -3328,6 +3344,12 @@
LIST_DEL(&h2s->list);
LIST_INIT(&h2s->list);
}
+ else if (LIST_ISEMPTY(&h2s->list)) {
+ if (h2s->flags & H2_SF_BLK_MFCTL)
+ LIST_ADDQ(&h2s->h2c->fctl_list, &h2s->list);
+ else if (h2s->flags & (H2_SF_BLK_MBUSY|H2_SF_BLK_MROOM))
+ LIST_ADDQ(&h2s->h2c->send_list, &h2s->list);
+ }
return total;
}