[PATCH] ngx_gzip might hang the request for slow downstreams

[Maxim Dounin]

Hello!

On Sat, Oct 26, 2013 at 05:16:38PM -0700, Yichun Zhang (agentzh) wrote:

> Hello!
> 
> On Sat, Oct 26, 2013 at 2:40 PM, Maxim Dounin wrote:
> >
> > Well, it would be good to adjust gzip (and other filters) to make
> > this actually work.  And I think something similar may be
> > triggered using non-buffered proxy, too.  Part of a response might
> > be left in buffers till new data from an upstream is received
> > (despite the fact that a client is again able to recieve the data,
> > and non-buffered proxy tries to flush everything as fast as
> > possible).
> >
> 
> Yeah, I was thinking along this line too ;)
> 
> > I don't think the patch you suggested is correct though.  At least
> > it lacks update chains call.  It may also make sense to look at
> > the "if (ctx->nomem)" codepath, which is very similar and probably
> > may be adjusted to handle this case as well.
> >
> 
> Thank you for the suggestions! How about the following patch?
> 
> Regards,
> -agentzh
> 
> --- nginx-1.4.3/src/http/modules/ngx_http_gzip_filter_module.c
> 2013-10-08 05:07:14.000000000 -0700
> +++ nginx-1.4.3-patched/src/http/modules/ngx_http_gzip_filter_module.c
> 2013-10-26 17:14:22.132109569 -0700
> @@ -370,11 +370,12 @@ ngx_http_gzip_body_filter(ngx_http_reque
>          }
>      }
> 
> -    if (ctx->nomem) {
> +    if (ctx->nomem || in == NULL) {
> 
>          /* flush busy buffers */
> 
> -        if (ngx_http_next_body_filter(r, NULL) == NGX_ERROR) {
> +        rc = ngx_http_next_body_filter(r, NULL);
> +        if (rc == NGX_ERROR) {
>              goto failed;
>          }

Just a side note: this change is unrelated.

> 
> @@ -383,6 +384,10 @@ ngx_http_gzip_body_filter(ngx_http_reque
>          ngx_chain_update_chains(r->pool, &ctx->free, &ctx->busy, &cl,
>                                  (ngx_buf_tag_t) &ngx_http_gzip_filter_module);
>          ctx->nomem = 0;
> +
> +        if (in == NULL) {
> +            return rc;
> +        }
>      }
> 
>      for ( ;; ) {


If there are pending data in ctx->in, they won't be handled.

-- 
Maxim Dounin
http://nginx.org/en/donation.html