BUG/MINOR: mux-h1: wake the connection up when a pending error can be sent

When h1_send_error() fails to allocate the output buffer, it sets
H1C_F_ABRT_PENDING and the connection is queued in the buffer wait list with
H1C_F_OUT_ALLOC. But when a buffer becomes available, h1_buf_available()
only handles H1C_F_OUT_ALLOC if an H1 stream exists, and then only wakes the
stream up for sending. When the error concerns a connection without H1
stream (e.g. a 408 on an idle connection or a 405 for an H2 preface received
on a kept-alive connection), or an embryonic H1 stream (no stream connector
to wake), nothing is done: the offered buffer is consumed for nothing,
H1C_F_OUT_ALLOC stays set so that any further attempt in h1_send_error()
gives up immediately, and the error is never sent. The connection then
lingers until its timeout.

In addition, even when the H1 connection's tasklet is woken up, h1_io_cb()
only calls h1_process() if some data were sent or received, so the pending
error would not be processed either.

Now, when an error is pending, h1_buf_available() clears the allocation flag
and wakes the connection's tasklet up, and h1_io_cb() always calls
h1_process() in this case so that the error is sent.

This can only happen when buffers allocation fails. The issue exists in some
form since the error handling on allocation failures was added, at least
since commit 56a499475 ("CLEANUP: mux-h1: Rename H1C_F_ERR_PENDING into
H1C_F_ABRT_PENDING") in 2.7. It may be backported as far as 2.8 after some
observation time.

diff --git a/src/mux_h1.c b/src/mux_h1.c
index 2c30ae8a6bd8fab6b38c34d2fc61fb9d7411d5bf..e24fd26f1304c8cd88835767c2475e62fa12928e 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -600,7 +600,17 @@ static int h1_buf_available(void *target)
                h1c->flags |=  H1C_F_IN_MAYALLOC;
        }
 
-       if ((h1c->flags & H1C_F_OUT_ALLOC) && h1c->h1s) {
+       if ((h1c->flags & H1C_F_OUT_ALLOC) && (h1c->flags & H1C_F_ABRT_PENDING)) {
+               /* an error message is waiting for the obuf, maybe without any
+                * stream (e.g. 408 on an idle connection), so the connection
+                * itself must be woken up to send it.
+                */
+               TRACE_STATE("unblocking h1c, obuf allocatable for pending error", H1_EV_H1C_ERR|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1c->conn);
+               h1c->flags &= ~H1C_F_OUT_ALLOC;
+               h1c->flags |=  H1C_F_OUT_MAYALLOC;
+               tasklet_wakeup(h1c->wait_event.tasklet);
+       }
+       else if ((h1c->flags & H1C_F_OUT_ALLOC) && h1c->h1s) {
                TRACE_STATE("unblocking h1s, obuf allocatable", H1_EV_TX_DATA|H1_EV_H1S_BLK|H1_EV_STRM_WAKE, h1c->conn, h1c->h1s);
                h1c->flags &= ~H1C_F_OUT_ALLOC;
                h1c->flags |=  H1C_F_OUT_MAYALLOC;
@@ -4539,7 +4549,7 @@ struct task *h1_io_cb(struct task *t, void *ctx, unsigned int state)
                ret = h1_send(h1c);
        if (!(h1c->wait_event.events & SUB_RETRY_RECV))
                ret |= h1_recv(h1c);
-       if (ret || b_data(&h1c->ibuf))
+       if (ret || b_data(&h1c->ibuf) || (h1c->flags & H1C_F_ABRT_PENDING))
                ret = h1_process(h1c);
 
        /* If we were in an idle list, we want to add it back into it,