On Sat, May 11, 2019 at 11:01:42AM +0200, Willy Tarreau wrote:
> On Sat, May 11, 2019 at 10:52:35AM +0200, Willy Tarreau wrote:
> > I certainly made a few reasoning mistakes above but I don't see anything
> > in the code preventing this case from happening.
> > 
> > Thus I'd like you to try the attached patch which is supposed to prevent
> > this scenario from happening. At least I've verified that it doesn't
> > break the h2spec test suite.
> 
> While trying to check if it still applied to the latest 1.9 I figured
> that it corresponds to what Olivier had also found and fixed in his
> latest patch :-/  The positive point is that my analysis was correct.
> 
> So I'm afraid that if it still fails with his fix, we'll need another
> core :-(

Actually not, Olivier's fix is incomplete regarding the scenario I proposed :
- in h2s_frt_make_resp_data() we can set H2_SF_BLK_SFCTL and remove the
  element from the 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.

Thus the first part of the patch I sent is still required, I'm attaching it
again, rebased on top of Olivier's patch and simplified so that we don't
detach then re-attach.

I'm still keeping hope ;-)

Willy
diff --git a/src/mux_h2.c b/src/mux_h2.c
index d201921..94e4e5e 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -1484,9 +1484,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);
@@ -1791,9 +1790,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 {

Reply via email to