BUG/MEDIUM: stream: Abort processing if response buffer allocation fails

In process_stream(), we force the response buffer allocation before any
processing to be able to return an error message. It is important because,
when an error is triggered, the stream is immediately closed. Thus we cannot
wait for the response buffer allocation.

When the allocation fails, the stream analysis is stopped and the expiration
date of the stream's task is updated before exiting process_stream(). But if
the stream was woken up because of a connection or an analysis timeout, the
expiration date remains blocked in the past. This means the stream is woken
up in loop as long as the response buffer is not properly allocated.

Alone, this behavior is already a bug. But because the mechanism to handle
buffer allocation failures is totally broken since a while, this bug becomes
more problematic. Because, most of time, the watchdog will kill HAProxy in
this case because it will detect a spinning loop.

To fix it, at least temporarily, an allocation failure at this stage is now
reported as an error and the processing is aborted. It's not satisfying but
it is better than nothing. If the buffers allocation mechanism is
refactored, this part will be reviewed.

This patch must be backported, probably as far as 2.0. It may be perceived
as a regression, but the actual behavior is probably even worse. And
because it was not reported, it is probably not a common situation.

diff --git a/src/stream.c b/src/stream.c
index d10b76e5daf4acd79d0dc7f446e7d6fbca09f238..628bdf56b60778b68d87c3401613b1e2074c841c 100644 (file)
--- a/src/stream.c
+++ b/src/stream.c
@@ -777,13 +777,8 @@ static void stream_free(struct stream *s)
  */
 static int stream_alloc_work_buffer(struct stream *s)
 {
-       if (LIST_INLIST(&s->buffer_wait.list))
-               LIST_DEL_INIT(&s->buffer_wait.list);
-
        if (b_alloc(&s->res.buf))
                return 1;
-
-       LIST_APPEND(&th_ctx->buffer_wq, &s->buffer_wait.list);
        return 0;
 }
 
@@ -1717,15 +1712,24 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
 
  resync_stream_interface:
        /* below we may emit error messages so we have to ensure that we have
-        * our buffers properly allocated.
+        * our buffers properly allocated. If the allocation failed, an error is
+        * triggered.
+        *
+        * NOTE: An error is returned because the mechanism to queue entities
+        *       waiting for a buffer is totally broken for now. However, this
+        *       part must be refactored. When it will be handled, this part
+        *       must be be reviewed too.
         */
        if (!stream_alloc_work_buffer(s)) {
-               /* No buffer available, we've been subscribed to the list of
-                * buffer waiters, let's wait for our turn.
-                */
-               si_f->flags &= ~SI_FL_DONT_WAKE;
-               si_b->flags &= ~SI_FL_DONT_WAKE;
-               goto update_exp_and_leave;
+               si_f->flags |= SI_FL_ERR;
+               si_f->err_type = SI_ET_CONN_RES;
+
+               si_b->flags |= SI_FL_ERR;
+               si_b->err_type = SI_ET_CONN_RES;
+
+               if (!(s->flags & SF_ERR_MASK))
+                       s->flags |= SF_ERR_RESOURCE;
+               sess_set_term_flags(s);
        }
 
        /* 1b: check for low-level errors reported at the stream interface.