From 4087346daba41aae557e5bea15c42e1d2ae9a818 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 13 May 2019 08:05:28 +0200 Subject: [PATCH] BUG/MAJOR: mux-h2: do not add a stream twice to the send list In this long thread, Maciej Zdeb reported that the H2 mux was still going through endless loops from time to time : https://www.mail-archive.com/haproxy@formilux.org/msg33709.html What happens is the following : - in h2s_frt_make_resp_data() we can set H2_SF_BLK_SFCTL and remove the stream from the send_list - then in h2_shutr() and h2_shutw(), we check if the list is empty before subscribing the element, which is true after the case above - then in h2c_update_all_ws() we still have H2_SF_BLK_SFCTL with the item in the send_list, thus LIST_ADDQ() adds it a second time. This patch adds a check of list emptiness before performing the LIST_ADDQ() when the flow control window opens. Maciej reported that it reliably fixed the problem for him. As later discussed with Olivier, this fixes the consequence of the issue rather than its cause. The root cause is that a stream should never be in the send_list with a blocking flag set and the various places that can lead to this situation must be revisited. Thus another fix is expected soon for this issue, which will require some observation. In the mean time this one is easy enough to validate and to backport. Many thanks to Maciej for testing several versions of the patch, each time providing detailed traces which allowed to nail the problem down. This patch must be backported to 1.9. --- src/mux_h2.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/mux_h2.c b/src/mux_h2.c index d1fa25a5d..608b73893 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -1498,9 +1498,8 @@ static void h2c_update_all_ws(struct h2c *h2c, int diff) if (h2s->mws > 0 && (h2s->flags & H2_SF_BLK_SFCTL)) { h2s->flags &= ~H2_SF_BLK_SFCTL; - if (h2s->send_wait) + if (h2s->send_wait && LIST_ISEMPTY(&h2s->list)) LIST_ADDQ(&h2c->send_list, &h2s->list); - } node = eb32_next(node); @@ -1805,9 +1804,8 @@ static int h2c_handle_window_update(struct h2c *h2c, struct h2s *h2s) h2s->mws += inc; if (h2s->mws > 0 && (h2s->flags & H2_SF_BLK_SFCTL)) { h2s->flags &= ~H2_SF_BLK_SFCTL; - if (h2s->send_wait) + if (h2s->send_wait && LIST_ISEMPTY(&h2s->list)) LIST_ADDQ(&h2c->send_list, &h2s->list); - } } else {