BUG/MEDIUM: mux-h2: permit the buffer callback to allocate despite the queue

Since 3.0 with commit d1eb48a12 ("MEDIUM: dynbuf: refrain from offering
a buffer if more critical ones are waiting"), b_alloc() refuses to
allocate a buffer when the wait queue of the same or more critical levels
is not empty, unless DB_F_NOQUEUE is passed. But h2_buf_available() is
called by offer_buffers() while the h2c is still in the queue, and it
tries to allocate without this flag. For the demux buffer (DB_MUX_RX,
which is also used by h2_get_buf() for all other buffers and is the
least critical queue), the h2c's own presence in the queue makes the
allocation always fail. The connection thus remains in the queue until
it times out, and as long as it's there, every DB_MUX_RX allocation on
the same thread (all H2 demux and mux buffers, H1 input buffers) has to
be queued as well and can only be served by buffer releases, so that
new connections on a quiet thread stall as well.

This requires a buffer allocation failure in the first place (memory
shortage or tune.buffers.limit reached), which is rare, but then a single
one is enough. It was reproduced with a debug patch faking one dbuf
allocation failure and calling offer_buffers() 200ms later on a single
thread: the callback sees bufq_map=0x8 and b_may_alloc_for_crit(DB_MUX_RX)
returns 0, and two H2 requests and a subsequent H1 request all hang for
5s until the client gives up, while they're all served in 200ms once the
flag is passed.

Let's pass DB_F_NOQUEUE to the allocations made from the callback, since
by definition we're called because a buffer is being offered to us. This
must be backported to 3.0.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 0c354c091a07f17a2e8c6ce9fa63de5eb8b28966..3a5aff4150e7796a95973950bc317542dd8cc381 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -1266,13 +1266,17 @@ static int h2_buf_available(void *target)
        struct h2c *h2c = target;
        struct h2s *h2s;
 
-       if ((h2c->flags & H2_CF_DEM_DALLOC) && b_alloc(&h2c->dbuf, DB_MUX_RX)) {
+       /* Note: we're still in the buffer wait queue when called, so we must
+        * pass DB_F_NOQUEUE to b_alloc() otherwise our own presence in the
+        * queue would make it refuse to allocate.
+        */
+       if ((h2c->flags & H2_CF_DEM_DALLOC) && b_alloc(&h2c->dbuf, DB_MUX_RX | DB_F_NOQUEUE)) {
                h2c->flags &= ~H2_CF_DEM_DALLOC;
                h2c_restart_reading(h2c, 1);
                return 1;
        }
 
-       if ((h2c->flags & H2_CF_MUX_MALLOC) && b_alloc(br_tail(h2c->mbuf), DB_MUX_TX)) {
+       if ((h2c->flags & H2_CF_MUX_MALLOC) && b_alloc(br_tail(h2c->mbuf), DB_MUX_TX | DB_F_NOQUEUE)) {
                h2c->flags &= ~H2_CF_MUX_MALLOC;
 
                if (h2c->flags & H2_CF_DEM_MROOM) {
@@ -1286,7 +1290,7 @@ static int h2_buf_available(void *target)
            (h2s = h2c_st_by_id(h2c, h2c->dsi)) && h2s_sc(h2s) &&
            (h2s_rxbuf_tail(h2s) || h2s_get_rxbuf(h2s))) {
                h2c->flags &= ~(H2_CF_DEM_RXBUF | H2_CF_DEM_SFULL);
-               if (b_alloc(_h2s_rxbuf_tail(h2s), DB_SE_RX)) {
+               if (b_alloc(_h2s_rxbuf_tail(h2s), DB_SE_RX | DB_F_NOQUEUE)) {
                        h2c->flags &= ~H2_CF_DEM_SALLOC;
                        h2c_restart_reading(h2c, 1);
                        return 1;