From da4d9fe5a428b2819929a53a4266c8f78a1fb147 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Sun, 7 Nov 2010 20:26:56 +0100 Subject: [PATCH] [BUG] session: don't stop forwarding of data upon last packet If a read shutdown is encountered on the first packet of a connection right after the data and the last analyser is unplugged at the same time, then that last data chunk may never be forwarded. In practice, right now it cannot happen on requests due to the way they're scheduled, nor can it happen on responses due to the way their analysers work. But this behaviour has been observed with new response analysers being developped. The reason is that when the read shutdown is encountered and an analyser is present, data cannot be forwarded but the BF_SHUTW_NOW flag is set. After that, the analyser gets called and unplugs itself, hoping that process_session() will automatically forward the data. This does not happen due to BF_SHUTW_NOW. Simply removing the test on this flag is not enough because then aborted requests still get forwarded, due to the forwarding code undoing the abort. The solution here consists in checking BF_SHUTR_NOW instead of BF_SHUTW_NOW. BF_SHUTR_NOW is only set on aborts and remains set until ->shutr() is called. This is enough to catch recent aborts but not prevent forwarding in other cases. Maybe a new special buffer flag "BF_ABORT" might be desirable in the future. This patch does not need to be backported because older versions don't have the analyser which make the problem appear. --- src/session.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/session.c b/src/session.c index 9f3ccec057..ae73c8ff4d 100644 --- a/src/session.c +++ b/src/session.c @@ -1703,9 +1703,11 @@ struct task *process_session(struct task *t) /* If noone is interested in analysing data, it's time to forward * everything. We configure the buffer to forward indefinitely. + * Note that we're checking BF_SHUTR_NOW as an indication of a possible + * recent call to buffer_abort(). */ if (!s->req->analysers && - !(s->req->flags & (BF_HIJACK|BF_SHUTW|BF_SHUTW_NOW)) && + !(s->req->flags & (BF_HIJACK|BF_SHUTW|BF_SHUTR_NOW)) && (s->req->prod->state >= SI_ST_EST) && (s->req->to_forward != BUF_INFINITE_FORWARD)) { /* This buffer is freewheeling, there's no analyser nor hijacker @@ -1717,11 +1719,10 @@ struct task *process_session(struct task *t) buffer_auto_close(s->req); buffer_flush(s->req); - /* If the producer is still connected, we'll enable data to flow - * from the producer to the consumer (which might possibly not be - * connected yet). + /* We'll let data flow between the producer (if still connected) + * to the consumer (which might possibly not be connected yet). */ - if (!(s->req->flags & (BF_SHUTR|BF_SHUTW|BF_SHUTW_NOW))) + if (!(s->req->flags & (BF_SHUTR|BF_SHUTW_NOW))) buffer_forward(s->req, BUF_INFINITE_FORWARD); } @@ -1825,9 +1826,11 @@ struct task *process_session(struct task *t) /* If noone is interested in analysing data, it's time to forward * everything. We configure the buffer to forward indefinitely. + * Note that we're checking BF_SHUTR_NOW as an indication of a possible + * recent call to buffer_abort(). */ if (!s->rep->analysers && - !(s->rep->flags & (BF_HIJACK|BF_SHUTW|BF_SHUTW_NOW)) && + !(s->rep->flags & (BF_HIJACK|BF_SHUTW|BF_SHUTR_NOW)) && (s->rep->prod->state >= SI_ST_EST) && (s->rep->to_forward != BUF_INFINITE_FORWARD)) { /* This buffer is freewheeling, there's no analyser nor hijacker @@ -1837,7 +1840,11 @@ struct task *process_session(struct task *t) buffer_auto_read(s->rep); buffer_auto_close(s->rep); buffer_flush(s->rep); - if (!(s->rep->flags & (BF_SHUTR|BF_SHUTW|BF_SHUTW_NOW))) + + /* We'll let data flow between the producer (if still connected) + * to the consumer. + */ + if (!(s->rep->flags & (BF_SHUTR|BF_SHUTW_NOW))) buffer_forward(s->rep, BUF_INFINITE_FORWARD); } -- 2.47.3