I think I found a fix for the memory leak issue on gRPC module
Sergey Kandaurov
pluknet at nginx.com
Mon May 27 13:04:30 UTC 2024
On Mon, May 27, 2024 at 02:06:58PM +0400, Roman Arutyunyan wrote:
> Hi,
> # HG changeset patch
> # User Roman Arutyunyan <arut at nginx.com>
> # Date 1716477338 -14400
> # Thu May 23 19:15:38 2024 +0400
> # Node ID f7d53c7f70140b1cd1eaf51ce4346a873692f879
> # Parent f58b6f6362387eeace46043a6fc0bceb56a6786a
> Optimized chain link usage (ticket #2614).
>
> Previously chain links could sometimes be dropped instead of being reused,
> which could result in increased memory consumption during long requests.
>
> A similar chain link issue in ngx_http_gzip_filter_module was fixed in
> da46bfc484ef (1.11.10).
>
> Based on a patch by Sangmin Lee.
>
> diff --git a/src/core/ngx_output_chain.c b/src/core/ngx_output_chain.c
> --- a/src/core/ngx_output_chain.c
> +++ b/src/core/ngx_output_chain.c
> @@ -117,7 +117,10 @@ ngx_output_chain(ngx_output_chain_ctx_t
>
> ngx_debug_point();
>
> - ctx->in = ctx->in->next;
> + cl = ctx->in;
> + ctx->in = cl->next;
> +
> + ngx_free_chain(ctx->pool, cl);
>
> continue;
> }
> @@ -203,7 +206,10 @@ ngx_output_chain(ngx_output_chain_ctx_t
> /* delete the completed buf from the ctx->in chain */
>
> if (ngx_buf_size(ctx->in->buf) == 0) {
> - ctx->in = ctx->in->next;
> + cl = ctx->in;
> + ctx->in = cl->next;
> +
> + ngx_free_chain(ctx->pool, cl);
> }
>
> cl = ngx_alloc_chain_link(ctx->pool);
> diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c
> --- a/src/http/modules/ngx_http_grpc_module.c
> +++ b/src/http/modules/ngx_http_grpc_module.c
> @@ -1231,7 +1231,7 @@ ngx_http_grpc_body_output_filter(void *d
> ngx_buf_t *b;
> ngx_int_t rc;
> ngx_uint_t next, last;
> - ngx_chain_t *cl, *out, **ll;
> + ngx_chain_t *cl, *out, *ln, **ll;
> ngx_http_upstream_t *u;
> ngx_http_grpc_ctx_t *ctx;
> ngx_http_grpc_frame_t *f;
> @@ -1459,7 +1459,10 @@ ngx_http_grpc_body_output_filter(void *d
> last = 1;
> }
>
> + ln = in;
> in = in->next;
> +
> + ngx_free_chain(r->pool, ln);
> }
Looks good now.
>
> ctx->in = in;
> diff --git a/src/http/modules/ngx_http_gunzip_filter_module.c b/src/http/modules/ngx_http_gunzip_filter_module.c
> --- a/src/http/modules/ngx_http_gunzip_filter_module.c
> +++ b/src/http/modules/ngx_http_gunzip_filter_module.c
> @@ -333,6 +333,8 @@ static ngx_int_t
> ngx_http_gunzip_filter_add_data(ngx_http_request_t *r,
> ngx_http_gunzip_ctx_t *ctx)
> {
> + ngx_chain_t *cl;
> +
> if (ctx->zstream.avail_in || ctx->flush != Z_NO_FLUSH || ctx->redo) {
> return NGX_OK;
> }
> @@ -344,8 +346,11 @@ ngx_http_gunzip_filter_add_data(ngx_http
> return NGX_DECLINED;
> }
>
> - ctx->in_buf = ctx->in->buf;
> - ctx->in = ctx->in->next;
> + cl = ctx->in;
> + ctx->in_buf = cl->buf;
> + ctx->in = cl->next;
> +
> + ngx_free_chain(r->pool, cl);
>
> ctx->zstream.next_in = ctx->in_buf->pos;
> ctx->zstream.avail_in = ctx->in_buf->last - ctx->in_buf->pos;
> @@ -374,6 +379,7 @@ static ngx_int_t
> ngx_http_gunzip_filter_get_buf(ngx_http_request_t *r,
> ngx_http_gunzip_ctx_t *ctx)
> {
> + ngx_chain_t *cl;
> ngx_http_gunzip_conf_t *conf;
>
> if (ctx->zstream.avail_out) {
> @@ -383,8 +389,12 @@ ngx_http_gunzip_filter_get_buf(ngx_http_
> conf = ngx_http_get_module_loc_conf(r, ngx_http_gunzip_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);
>
> ctx->out_buf->flush = 0;
>
> 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
> @@ -985,10 +985,14 @@ static void
> ngx_http_gzip_filter_free_copy_buf(ngx_http_request_t *r,
> ngx_http_gzip_ctx_t *ctx)
> {
> - ngx_chain_t *cl;
> + ngx_chain_t *cl, *ln;
>
> - for (cl = ctx->copied; cl; cl = cl->next) {
> - ngx_pfree(r->pool, cl->buf->start);
> + for (cl = ctx->copied; cl; /* void */) {
> + ln = cl;
> + cl = cl->next;
> +
> + ngx_pfree(r->pool, ln->buf->start);
> + ngx_free_chain(r->pool, ln);
> }
>
> ctx->copied = NULL;
> diff --git a/src/http/modules/ngx_http_ssi_filter_module.c b/src/http/modules/ngx_http_ssi_filter_module.c
> --- a/src/http/modules/ngx_http_ssi_filter_module.c
> +++ b/src/http/modules/ngx_http_ssi_filter_module.c
> @@ -482,9 +482,13 @@ ngx_http_ssi_body_filter(ngx_http_reques
> while (ctx->in || ctx->buf) {
>
> if (ctx->buf == NULL) {
> - ctx->buf = ctx->in->buf;
> - ctx->in = ctx->in->next;
> +
> + cl = ctx->in;
> + ctx->buf = cl->buf;
> + ctx->in = cl->next;
> ctx->pos = ctx->buf->pos;
> +
> + ngx_free_chain(r->pool, cl);
> }
>
> if (ctx->state == ssi_start_state) {
> diff --git a/src/http/modules/ngx_http_sub_filter_module.c b/src/http/modules/ngx_http_sub_filter_module.c
> --- a/src/http/modules/ngx_http_sub_filter_module.c
> +++ b/src/http/modules/ngx_http_sub_filter_module.c
> @@ -335,9 +335,13 @@ ngx_http_sub_body_filter(ngx_http_reques
> while (ctx->in || ctx->buf) {
>
> if (ctx->buf == NULL) {
> - ctx->buf = ctx->in->buf;
> - ctx->in = ctx->in->next;
> +
> + cl = ctx->in;
> + ctx->buf = cl->buf;
> + ctx->in = cl->next;
> ctx->pos = ctx->buf->pos;
> +
> + ngx_free_chain(r->pool, cl);
> }
>
> if (ctx->buf->flush || ctx->buf->recycled) {
More information about the nginx-devel
mailing list