Opened 8 years ago

Closed 8 years ago

#1046 closed defect (fixed)

ngx_http_gzip_filter_module is leaking

Reported by: burkov@… Owned by:
Priority: major Milestone:
Component: nginx-module Version: 1.10.x
Keywords: gzip, memory, leakage Cc:
uname -a: Linux <hostname> 4.4.0-28-generic #47-Ubuntu SMP Fri Jun 24 10:09:13 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
nginx -V: nginx version: nginx/1.10.1
built with OpenSSL 1.0.2g-fips 1 Mar 2016
TLS SNI support enabled
configure arguments: --with-cc-opt='-g -O2 -fPIE -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2' --with-ld-opt='-Wl,-Bsymbolic-functions -fPIE -pie -Wl,-z,relro -Wl,-z,now' --prefix=/usr/share/nginx --conf-path=/etc/nginx/nginx.conf --http-log-path=/var/log/nginx/access.log --error-log-path=/var/log/nginx/error.log --lock-path=/var/lock/nginx.lock --pid-path=/run/nginx.pid --http-client-body-temp-path=/var/lib/nginx/body --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-proxy-temp-path=/var/lib/nginx/proxy --http-scgi-temp-path=/var/lib/nginx/scgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --with-debug --with-pcre-jit --with-ipv6 --with-http_ssl_module --with-http_stub_status_module --with-http_realip_module --with-http_auth_request_module --with-http_addition_module --with-http_dav_module --with-http_flv_module --with-http_geoip_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_image_filter_module --with-http_mp4_module --with-http_perl_module --with-http_random_index_module --with-http_secure_link_module --with-http_v2_module --with-http_sub_module --with-http_xslt_module --with-mail --with-mail_ssl_module --with-stream --with-stream_ssl_module --with-threads --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/headers-more-nginx-module --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/nginx-auth-pam --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/nginx-cache-purge --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/nginx-dav-ext-module --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/nginx-development-kit --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/nginx-echo --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/ngx-fancyindex --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/nginx-http-push --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/nginx-lua --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/nginx-upload-progress --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/nginx-upstream-fair --add-module=/build/nginx-3I6R0c/nginx-1.10.1/debian/modules/ngx_http_substitutions_filter_module

Description

It seems that gzip filter module is leaking. I'm using nginx for gzipping my stream server data:

http {

  gzip on;
  gzip_comp_level 1;
  gzip_proxied any;
  gzip_types application/octet-stream;

  server {
    listen 8080;
    chunked_transfer_encoding on;
    location /stream {
      proxy_buffering off;
      proxy_pass http://localhost:8081;
    }
  }
}

In this example http://localhost:8081 sends chunked http response of infinite size on every request

I've tried to estimate this issue debugging ngx_http_filter_module.c but was not able to fix it. It looks like ctx->request->pool is leaking (?), every new ngx_palloc(..) on it gives increasing memory addresses. After 24+ hours of such workload nginx workers RSS on my server takes up to 3-4 Gb of RAM

I've done a small app which show the problem: https://github.com/burkov/gzip_streamer
You'll need JDK to compile and run it. Hope this help.

Change History (5)

comment:1 by Valentin V. Bartenev, 8 years ago

That is expected behaviour. Nginx accumulates small allocations to free them as a bunch at the end of processing. The lifetime of ctx->request->pool is equal to the request processing time, which is usually many times shorter than 24+ hours. It doesn't optimized for your workload.

in reply to:  1 comment:2 by burkov@…, 8 years ago

Replying to vbart:

That is expected behaviour. Nginx accumulates small allocations to free them as a bunch at the end of processing. The lifetime of ctx->request->pool is equal to the request processing time, which is usually many times shorter than 24+ hours. It doesn't optimized for your workload.

But when I disable gzipping and use nginx only as a reverse proxy for the same backend it doesn't leak. No matter how long a connection exists. Seems like the problem is only in the gzip module.

Last edited 8 years ago by burkov@… (previous) (diff)

comment:3 by Maxim Dounin, 8 years ago

Just to clarify: there is no leak here. All the memory allocates is bound to a particular request, and will be freed with the request. This is the pool memory module nginx uses to avoid leaks: it does all allocations from the request pool, and frees everything with the request itself. This model makes it almost impossible to introduce memory leaks, but may use a lot of memory in the edge case of long-running requests, as in your case.

To minimize negative effects of this model, some common code paths are carefully coded to reuse all the allocated memory. And this is what you can seen with gzip disabled: even long-running requests do not consume more memory over time. This is not always the case though, and various workloads may consume more memory over time if requests are long-running. Note well that long running requests also have other problems - for example, they won't allow old nginx workers to exit after a configuration reload. It is generally a good idea to keep requests short enough.

Looking into gzip I see that on typical code path it doesn't reuse chain links (ngx_chain_t structures) when working with input buffers, and when obtaining free buffers. With default buffer sizes and assuming non-compressible input, this means about 32 bytes per 4k transferred on 64-bit platforms. So it will require about 8M to transfer a 1G response. Not that much, actually, but certainly can be seen in long-running tests.

The following patch fixes this:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1486942110 -10800
#      Mon Feb 13 02:28:30 2017 +0300
# Node ID 978a48c977b71257a5b755d6a8c13dbb18c209bd
# Parent  5850fed2463904e9bc17d37e4df3ae401b66048f
Gzip filter: free chain links on the hot path (ticket #1046).

diff --git a/src/http/modules/ngx_http_gzip_filter_module.c b/src/http/modules/ngx_http_gzip_filter_module.c
--- a/src/http/modules/ngx_http_gzip_filter_module.c
+++ b/src/http/modules/ngx_http_gzip_filter_module.c
@@ -671,6 +671,8 @@ ngx_http_gzip_filter_gzheader(ngx_http_r
 static ngx_int_t
 ngx_http_gzip_filter_add_data(ngx_http_request_t *r, ngx_http_gzip_ctx_t *ctx)
 {
+    ngx_chain_t  *cl;
+
     if (ctx->zstream.avail_in || ctx->flush != Z_NO_FLUSH || ctx->redo) {
         return NGX_OK;
     }
@@ -694,14 +696,17 @@ ngx_http_gzip_filter_add_data(ngx_http_r
         ctx->copy_buf = NULL;
     }
 
-    ctx->in_buf = ctx->in->buf;
+    cl = ctx->in;
+    ctx->in_buf = cl->buf;
+    ctx->in = cl->next;
 
     if (ctx->in_buf->tag == (ngx_buf_tag_t) &ngx_http_gzip_filter_module) {
-        ctx->copy_buf = ctx->in;
+        ctx->copy_buf = cl;
+
+    } else {
+        ngx_free_chain(r->pool, cl);
     }
 
-    ctx->in = ctx->in->next;
-
     ctx->zstream.next_in = ctx->in_buf->pos;
     ctx->zstream.avail_in = ctx->in_buf->last - ctx->in_buf->pos;
 
@@ -733,6 +738,7 @@ ngx_http_gzip_filter_add_data(ngx_http_r
 static ngx_int_t
 ngx_http_gzip_filter_get_buf(ngx_http_request_t *r, ngx_http_gzip_ctx_t *ctx)
 {
+    ngx_chain_t           *cl;
     ngx_http_gzip_conf_t  *conf;
 
     if (ctx->zstream.avail_out) {
@@ -742,8 +748,12 @@ ngx_http_gzip_filter_get_buf(ngx_http_re
     conf = ngx_http_get_module_loc_conf(r, ngx_http_gzip_filter_module);
 
     if (ctx->free) {
-        ctx->out_buf = ctx->free->buf;
-        ctx->free = ctx->free->next;
+
+        cl = ctx->free;
+        ctx->out_buf = cl->buf;
+        ctx->free = cl->next;
+
+        ngx_free_chain(r->pool, cl);
 
     } else if (ctx->bufs < conf->bufs.num) {
 

comment:4 by Maxim Dounin <mdounin@…>, 8 years ago

In 6910:da46bfc484ef/nginx:

Gzip: free chain links on the hot path (ticket #1046).

comment:5 by Maxim Dounin, 8 years ago

Resolution: fixed
Status: newclosed

Fix committed.

Note: See TracTickets for help on using tickets.