You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.

88 lines
3.5 KiB

  1. From 9154bc92ed11c6de75573dec341b6a0ce68bd0eb Mon Sep 17 00:00:00 2001
  2. From: Willy Tarreau <w@1wt.eu>
  3. Date: Wed, 25 Nov 2015 20:17:27 +0100
  4. Subject: [PATCH 07/10] BUG/MEDIUM: stream: fix half-closed timeout handling
  5. client-fin and server-fin are bogus. They are applied on the write
  6. side after a SHUTR was seen. The immediate effect is that sometimes
  7. if a SHUTR was seen after a SHUTW on the same side, the timeout is
  8. enabled again regardless of the fact that the output is already
  9. closed. This results in the timeout event not to be processed and
  10. a busy poll loop to happen until another timeout on the stream gets
  11. rid of it. Note that haproxy continues its job during this, it's just
  12. that it eats all the CPU trying to handle an event that it ignores.
  13. An reproducible case consists in having a client stop reading data from
  14. a server to ensure data remain in the response buffer, then the client
  15. sends a shutdown(write). If abortonclose is enabled on haproxy, the
  16. shutdown is passed to the server side and the server responds with a
  17. SHUTR that cannot immediately be forwarded to the client since the
  18. buffer is full. During this time the event is ignored and the task is
  19. woken again in loops.
  20. It is worth noting that the timeout handling since 1.5 is a bit fragile
  21. and that it might be possible that other similar conditions still exist,
  22. so the timeout handling should be audited regarding this issue.
  23. Many thanks to BaiYang for providing detailed information showing the
  24. problem in action.
  25. This bug also affects 1.5 thus the fix must be backported.
  26. (cherry picked from commit f25b3573d65fd2411c7537b7b0a4817b478df909)
  27. [Note for 1.5, it's in session.c here]
  28. (cherry picked from commit 44e86286159474a52dc74f80d3271504cc6f1550)
  29. ---
  30. src/session.c | 16 ----------------
  31. 1 file changed, 16 deletions(-)
  32. diff --git a/src/session.c b/src/session.c
  33. index 7520a85..2b2ad78 100644
  34. --- a/src/session.c
  35. +++ b/src/session.c
  36. @@ -2213,10 +2213,6 @@ struct task *process_session(struct task *t)
  37. if (unlikely((s->req->flags & (CF_SHUTW|CF_SHUTW_NOW|CF_AUTO_CLOSE|CF_SHUTR)) ==
  38. (CF_AUTO_CLOSE|CF_SHUTR))) {
  39. channel_shutw_now(s->req);
  40. - if (tick_isset(s->fe->timeout.clientfin)) {
  41. - s->rep->wto = s->fe->timeout.clientfin;
  42. - s->rep->wex = tick_add(now_ms, s->rep->wto);
  43. - }
  44. }
  45. /* shutdown(write) pending */
  46. @@ -2241,10 +2237,6 @@ struct task *process_session(struct task *t)
  47. if (s->req->prod->flags & SI_FL_NOHALF)
  48. s->req->prod->flags |= SI_FL_NOLINGER;
  49. si_shutr(s->req->prod);
  50. - if (tick_isset(s->fe->timeout.clientfin)) {
  51. - s->rep->wto = s->fe->timeout.clientfin;
  52. - s->rep->wex = tick_add(now_ms, s->rep->wto);
  53. - }
  54. }
  55. /* it's possible that an upper layer has requested a connection setup or abort.
  56. @@ -2391,10 +2383,6 @@ struct task *process_session(struct task *t)
  57. if (unlikely((s->rep->flags & (CF_SHUTW|CF_SHUTW_NOW|CF_AUTO_CLOSE|CF_SHUTR)) ==
  58. (CF_AUTO_CLOSE|CF_SHUTR))) {
  59. channel_shutw_now(s->rep);
  60. - if (tick_isset(s->be->timeout.serverfin)) {
  61. - s->req->wto = s->be->timeout.serverfin;
  62. - s->req->wex = tick_add(now_ms, s->req->wto);
  63. - }
  64. }
  65. /* shutdown(write) pending */
  66. @@ -2417,10 +2405,6 @@ struct task *process_session(struct task *t)
  67. if (s->rep->prod->flags & SI_FL_NOHALF)
  68. s->rep->prod->flags |= SI_FL_NOLINGER;
  69. si_shutr(s->rep->prod);
  70. - if (tick_isset(s->be->timeout.serverfin)) {
  71. - s->req->wto = s->be->timeout.serverfin;
  72. - s->req->wex = tick_add(now_ms, s->req->wto);
  73. - }
  74. }
  75. if (s->req->prod->state == SI_ST_DIS || s->req->cons->state == SI_ST_DIS)
  76. --
  77. 2.4.10