git.99rst.org Git - openwrt-packages.git/blob

   1 From 4f2bd42ed3870dbaf143701f0cfbd64966d44252 Mon Sep 17 00:00:00 2001
   2 From: Willy Tarreau <w@1wt.eu>
   3 Date: Thu, 29 Mar 2018 15:22:59 +0200
   4 Subject: [PATCH] BUG/MEDIUM: h2/threads: never release the task outside of
   5  the task handler
   6
   7 Currently, h2_release() will release all resources assigned to the h2
   8 connection, including the timeout task if any. But since the multi-threaded
   9 scheduler, the timeout task could very well be queued in the thread-local
  10 list of running tasks without any way to remove it, so task_delete() will
  11 have no effect and task_free() will cause this undefined object to be
  12 dereferenced.
  13
  14 In order to prevent this from happening, we never release the task in
  15 h2_release(), instead we wake it up after marking its context NULL so that
  16 the task handler can release the task.
  17
  18 Future improvements could consist in modifying the scheduler so that a
  19 task_wakeup() has to be done on any task having to be killed, letting
  20 the scheduler take care of it.
  21
  22 This fix must be backported to 1.8. This bug was apparently not reported
  23 so far.
  24
  25 (cherry picked from commit 0975f11d554baf30602ce4be3faf0b9741711a80)
  26 Signed-off-by: Willy Tarreau <w@1wt.eu>
  27 ---
  28  src/mux_h2.c |   30 +++++++++++++++++-------------
  29  1 file changed, 17 insertions(+), 13 deletions(-)
  30
  31 diff --git a/src/mux_h2.c b/src/mux_h2.c
  32 index 3c076d2..92fae06 100644
  33 --- a/src/mux_h2.c
  34 +++ b/src/mux_h2.c
  35 @@ -484,8 +484,8 @@ static void h2_release(struct connection *conn)
  36                 HA_SPIN_UNLOCK(BUF_WQ_LOCK, &buffer_wq_lock);
  37
  38                 if (h2c->task) {
  39 -                       task_delete(h2c->task);
  40 -                       task_free(h2c->task);
  41 +                       h2c->task->context = NULL;
  42 +                       task_wakeup(h2c->task, TASK_WOKEN_OTHER);
  43                         h2c->task = NULL;
  44                 }
  45
  46 @@ -2369,9 +2369,18 @@ static struct task *h2_timeout_task(struct task *t)
  47         struct h2c *h2c = t->context;
  48         int expired = tick_is_expired(t->expire, now_ms);
  49
  50 -       if (!expired)
  51 +       if (!expired && h2c)
  52                 return t;
  53
  54 +       task_delete(t);
  55 +       task_free(t);
  56 +
  57 +       if (!h2c) {
  58 +               /* resources were already deleted */
  59 +               return NULL;
  60 +       }
  61 +
  62 +       h2c->task = NULL;
  63         h2c_error(h2c, H2_ERR_NO_ERROR);
  64         h2_wake_some_streams(h2c, 0, 0);
  65
  66 @@ -2388,17 +2397,12 @@ static struct task *h2_timeout_task(struct task *t)
  67         if (h2c->mbuf->o && !(h2c->flags & H2_CF_GOAWAY_FAILED) && conn_xprt_ready(h2c->conn))
  68                 h2c->conn->xprt->snd_buf(h2c->conn, h2c->mbuf, 0);
  69
  70 -       if (!eb_is_empty(&h2c->streams_by_id))
  71 -               goto wait;
  72 -
  73 -       h2_release(h2c->conn);
  74 -       return NULL;
  75 +       /* either we can release everything now or it will be done later once
  76 +        * the last stream closes.
  77 +        */
  78 +       if (eb_is_empty(&h2c->streams_by_id))
  79 +               h2_release(h2c->conn);
  80
  81 - wait:
  82 -       /* the streams have been notified, we must let them finish and close */
  83 -       h2c->task = NULL;
  84 -       task_delete(t);
  85 -       task_free(t);
  86         return NULL;
  87  }
  88
  89 --
  90 1.7.10.4
  91