I think I found a fix for the memory leak issue on gRPC module

[Pavel Pautov]

Hi Sangmin,

Your analysis looks correct. Client streaming RPCs can lead to unbound accumulation of ngx_chain_t objects (although, at very slow rate). Gzip module had a similar issue (https://trac.nginx.org/nginx/ticket/1046).
Attaching somewhat simplified patch based on yours. I was able to reproduce the issue locally and the patch fixes it.

From: nginx-devel <nginx-devel-bounces at nginx.org> On Behalf Of Sangmin Lee
Sent: Thursday, April 4, 2024 19:29
To: nginx-devel at nginx.org
Subject: I think I found a fix for the memory leak issue on gRPC module

CAUTION: This email has been sent from an external source. Do not click links, open attachments, or provide sensitive business information unless you can verify the sender's legitimacy.

I am sending this mail again because I did a mistake while I was writing a mail. I didn't know, in gmail, "Ctrl - Enter" would send a mail immediately even without a title!
I am sorry for that, so I am sending again.

Hello,

I think I found the main cause of the memory leak issue when using gRPC stream so I made a patch for it.
Please find the test scenario and details here -- This is what I wrote.:
https://trac.nginx.org/nginx/ticket/2614
After I changed the memory pool totally on nginx and tested our workload -- long-lived gRPC streams with many connections -- using Valgrind and massif, I was able to find what brought up the memory leak issue.
like the picture below.

[cid:image001.png at 01DA9C29.C792CD90]
( I am not sure whether this picture will be sent properly )

After I patched one part, it seems okay now I have tested it for 1 week with out workload.

[cid:image002.png at 01DA9C29.C792CD90]
( I am not sure whether this picture will be sent properly )

But because I am not familiar with Mercurial, I couldn't find a way to create PR like on github. I guess this mailing list is for this patch.
>From my point of view, it is more like a workaround and I think the way of using ngx_chain_add_copy() or itself needs to be changed because it allocates a ngx_chain_t structure using ngx_alloc_chain_link() but inside of that, it just copies pointer, like cl->buf = in->buf;
so this ngx_chain_t instance should be dealt with differently unlike other ngx_chain_t instances.
But I am quite new to nginx codes so my view might be wrong.
Anyhow, please go over this patch and I would like to further talk here.

--------------------------------------------------------------------------------------------------------------------------------------------

diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c
index dfe49c586..1db67bd0a 100644
--- a/src/http/modules/ngx_http_grpc_module.c
+++ b/src/http/modules/ngx_http_grpc_module.c
@@ -1462,6 +1462,12 @@ ngx_http_grpc_body_output_filter(void *data, ngx_chain_t *in)
         in = in->next;
     }

+       ngx_chain_t *nl;
+       for (ngx_chain_t *dl = ctx->in; dl != in; dl = nl ) {
+               nl = dl->next;
+               ngx_free_chain(r->pool, dl);
+       }
+
     ctx->in = in;

     if (last) {

--------------------------------------------------------------------------------------------------------------------------------------------

Best regards,
Sangmin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20240502/2bef2d65/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.png
Type: image/png
Size: 371470 bytes
Desc: image001.png
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20240502/2bef2d65/attachment-0002.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image002.png
Type: image/png
Size: 65011 bytes
Desc: image002.png
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20240502/2bef2d65/attachment-0003.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: grpc_leak.patch
Type: application/octet-stream
Size: 718 bytes
Desc: grpc_leak.patch
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20240502/2bef2d65/attachment-0001.obj>