Hello! On Mon, Nov 3, 2014 at 4:54 PM, Maxim Dounin wrote: > The commit log in question explains the reason for the change. > Work on the gzip stalls problem as fixed by 973fded4f461 clearly > showed that just passing NULL chains is wrong unless last buffer > was already sent or there are busy buffers.
AFAIK, this is the output filter caller's responsibility to ensure the requirements "last buffer was already sent or there are busy buffers", not every individual output body filter. Almost all the other standard nginx output filter modules including but not limited to ngx_addition, ngx_sub_filter, ngx_image_filter, ngx_chunked_filter, and ngx_xslt_filter. You meant to fix these modules as well? Also, the requirement "there are busy buffers" should not just be busy buffers of the output filter module itself, but rather busy buffers in all the other filters along the filter chain, include the busy bufs in r->out which is managed by ngx_http_write_filter_module at the bottom of the filter chain. > If you see an issue, you may want to share the issue details, > to find out how to fix it properly. The very issue of nginx 1.7.7 caught by my ngx_lua's test suite is in this condition: 1. the content handler runs out of buffers (i.e., having the maximum number of busy buffers), and 2. ngx_gzip or ngx_gunzip does not have any busy buffers for itself (i.e., no ctx->busy), and 3. ngx_http_write_filter_module has pending data for flush to the system send buffer, that is, having busy bufs in r->out. Upon new write events, I pass NULL to the output filter chain in order to flush the busy bufs of ngx_http_write_fiilter_module (in r->out), but ngx_gzip and ngx_gunzip quietly swallows it, wasting the write events, and leading to request hang. This use case of NULL chain in ngx_http_output_filter should considered valid because there are indeed busy bufs for ngx_http_write_filter_module (though not ngx_gzip/ngx_gunzip itself). Disabling the gzip or gunzip filter makes the problem go away immediately. Regarding the "output chain is empty" alert (generated by ngx_http_write_filter_module), it should only happen when there is *no* busy bufs and no data in r->out, which is apparently not the case explained above. In conclusion, your latest change does not even allow NULL flushing cases that perfectly *qualify* your own definition of the requirements "last buffer was already sent or there are busy buffers" because you explicitly prohibit flushing when there actually *is* busy bufs in ngx_http_write_filter_module, and just in ngx_gzip and ngx_gunzip (but not in any other output filter modules). Just my 2 cents :) Regards, -agentzh _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
