attached patch this time.
On Wed, Apr 03, 2013 at 12:32:14AM +0200, Willy Tarreau wrote:
> On Wed, Apr 03, 2013 at 12:11:35AM +0200, Willy Tarreau wrote:
> > Hi Sander,
> >
> > On Tue, Apr 02, 2013 at 11:35:45PM +0200, Willy Tarreau wrote:
> > > Sounds like I broke the response forward state machine last evening
> > > then (the last 3 patches that went into 20130402), because all other
> > > patches are just cosmetic. I'm re-auditing the code now.
> >
> > OK I could reproduce using nginx only as well. I still don't know what
> > the issue is, but I can already confirm that reverting commit d655ffe8
> > fixes the issue here.
>
> And here is the fix. I indeed messed up one point. Everything was OK in
> the code... except that we need to loop one last time before finishing
> the transfer, in order to flush pending data. This loop is still ugly
> and I'd really like to attack it but it scares me and you now understand
> why :-/
>
> I've pushed the fix as commit 2d43e18b, for those who want to pick it.
> Now I'm back finishing the ACL/sample merge.
>
> !hanks for reporting this!
> Willy
>
commit 2d43e18b6927b73aa9b1e6c8a40fc5b235498a5f
Author: Willy Tarreau <[email protected]>
Date: Wed Apr 3 00:22:25 2013 +0200
BUG/MAJOR: http: fix regression introduced by commit d655ffe
Sander Klein reported that since last snapshot, some downloads would
hang from nginx but succeed from apache. The culprit was not too hard
to find given the low number of recent changes affecting the data path.
Commit d655ffe slightly reorganized the HTTP state machine and
introduced this regression. The reason is that we must never jump
into the MSG_DONE case without first flushing remaining data because
this is not done anymore afterwards. This part is scheduled for
being reorganized since it's totally ugly especially since we added
compression, and this regression is an illustration of its readability.
The issue is entirely dependant on the server close sequence, which
explains why it was reproducible only with nginx here.
diff --git a/src/proto_http.c b/src/proto_http.c
index 311268f..f4502bf 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -5936,7 +5936,10 @@ int http_response_forward_body(struct session *s, struct
channel *res, int an_bi
channel_forward(res, msg->next);
msg->next = 0;
}
- /* we're in HTTP_MSG_DONE now, fall through */
+ /* we're in HTTP_MSG_DONE now, but we might still have
+ * some data pending, so let's loop over once.
+ */
+ break;
default:
/* other states, DONE...TUNNEL */
