BUG/MINOR: mux-h2: unblock the demux when destroying the stream it was blocked on

When the demux is blocked on a stream whose rxbufs are full, it sets
H2_CF_DEM_SFULL and waits for h2_rcv_buf() to make room. If the upper
layer leaves at this moment, h2_detach() normally clears the demux
blocking flags when the stream is the one being demuxed. But if the
stream could not send its RST_STREAM (or ES) at shutdown time because the
mux buffer was full, h2_detach() leaves early to keep the stream as an
orphan, and skips that part. The stream is later destroyed by
h2_deferred_shut() once the frame can be sent, but h2s_destroy() only
unblocks the demux if it was waiting for rxbuf slots (H2_CF_DEM_RXBUF),
not if it was waiting for this stream's buffers to be drained
(H2_CF_DEM_SFULL). The demux thus remains blocked until the timeout
triggers.

This requires an upload on a stream whose upper layer aborts (e.g. server
timeout or error) while the client is not reading its responses fast
enough for the RST_STREAM to be sent. It was reproduced with a debug patch
simulating a full mux buffer when sending the RST_STREAM of the stream
the demux is blocked on, with an H2 client uploading to a server which
never reads (timeout server 2s), then sending a GET on another stream of
the same connection: the GET never gets a response, while it gets a 200
immediately with the fix.

The fix consists in also clearing H2_CF_DEM_SFULL in h2s_destroy() when
the destroyed stream is the one the demux is blocked on. The issue seems
to exist since orphaned streams are destroyed from h2_deferred_shut()
(1.9). The fix may be backported to all supported versions, though before
3.1 the rxbuf release code differs and the same test must be applied after
releasing the stream's rxbuf.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 70426c52987c0b8fa0d4ff6c37b3138f3c6ee764..d1ce6351ff1be56661ef15251c4a99ce1a4cbf88 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2112,8 +2112,11 @@ static void h2s_destroy(struct h2s *h2s)
 
        if (freed) {
                offer_buffers(NULL, freed);
-               if (h2s->h2c->flags & H2_CF_DEM_RXBUF) {
-                       /* just released resources the demux is waiting for */
+               if ((h2s->h2c->flags & H2_CF_DEM_RXBUF) ||
+                   ((h2s->h2c->flags & H2_CF_DEM_SFULL) && h2s->id == h2s->h2c->dsi)) {
+                       /* just released resources the demux is waiting for,
+                        * either rxbuf slots or this stream's full rxbuf.
+                        */
                        h2s->h2c->flags &= ~(H2_CF_DEM_SFULL | H2_CF_DEM_RXBUF);
                        h2c_restart_reading(h2s->h2c, 1);
                }