[PATCH] ngx_gzip might hang the request for slow downstreams

[Yichun Zhang (agentzh)]

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;
         }

@@ -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 ( ;; ) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nginx-1.4.3-gzip_flush_bug.patch
Type: text/x-patch
Size: 901 bytes
Desc: not available
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20131026/d6657bcc/attachment.bin>