search

Kong / ngx_wasm_module

Nginx + WebAssembly
Apache License 2.0
79 stars 7 forks source link

fix(*) resolve a potential memleak in Lua bridge resolver #571

Closed thibaultcha closed 2 months ago

thibaultcha commented 2 months ago

An occasional memory leak of Proxy-Wasm dispatch with Lua bridge resolver, spotted by CI:

https://github.com/Kong/ngx_wasm_module/actions/runs/9884094484/job/27299927250

==19824== 224 bytes in 1 blocks are definitely lost in loss record 13 of 33
==19824==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==19824==    by 0x1E56AA: ngx_alloc (ngx_alloc.c:22)
==19824==    by 0x1CF926: ngx_resolver_alloc (ngx_resolver.c:4400)
==19824==    by 0x1CF926: ngx_resolver_calloc.isra.0 (ngx_resolver.c:4413)
==19824==    by 0x1D1EFD: ngx_resolve_start (ngx_resolver.c:661)
==19824==    by 0x392C80: ngx_wasm_socket_tcp_connect (ngx_wasm_socket_tcp.c:360)
==19824==    by 0x3BC984: ngx_http_proxy_wasm_dispatch_resume_handler (ngx_http_proxy_wasm_dispatch.c:745)
==19824==    by 0x3BD4C3: ngx_http_proxy_wasm_dispatch_handler (ngx_http_proxy_wasm_dispatch.c:477)
==19824==    by 0x1DFBE9: ngx_event_process_posted (ngx_event_posted.c:35)
==19824==    by 0x1DF0E6: ngx_process_events_and_timers (ngx_event.c:273)
==19824==    by 0x1EE697: ngx_single_process_cycle (ngx_process_cycle.c:323)
==19824==    by 0x1A9078: main (nginx.c:384)

This was caused by having 2 parallel dispatch calls using pwm_lua_resolver DNS resolution. When the first call responds and produces a downstream response, it causes pending calls to be cancelled.

However when a cancelled call had not yet finished the Lua-bridge DNS resolution, its associated rslv_ctx was never freed.

This adds a "cancel" feature to Lua bridge threads, which is invoked by TCP sockets when they are closed while having a pending Lua resolver thread.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 96.15385% with 2 lines in your changes missing coverage. Please review.

Project coverage is 90.59383%. Comparing base (cce2457) to head (190441e).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571/graphs/tree.svg?width=650&height=150&src=pr&token=T0PT2Q9IAN&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong)](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) ```diff @@ Coverage Diff @@ ## main #571 +/- ## =================================================== + Coverage 90.56413% 90.59383% +0.02969% =================================================== Files 49 49 Lines 10884 10929 +45 =================================================== + Hits 9857 9901 +44 - Misses 1027 1028 +1 ``` | [Files](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | Coverage Δ | | |---|---|---| | [src/common/lua/ngx\_wasm\_lua\_resolver.c](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571?src=pr&el=tree&filepath=src%2Fcommon%2Flua%2Fngx_wasm_lua_resolver.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong#diff-c3JjL2NvbW1vbi9sdWEvbmd4X3dhc21fbHVhX3Jlc29sdmVyLmM=) | `75.55556% <100.00000%> (-0.26862%)` | :arrow_down: | | [src/common/ngx\_wasm\_socket\_tcp.c](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571?src=pr&el=tree&filepath=src%2Fcommon%2Fngx_wasm_socket_tcp.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong#diff-c3JjL2NvbW1vbi9uZ3hfd2FzbV9zb2NrZXRfdGNwLmM=) | `89.46648% <100.00000%> (+0.07254%)` | :arrow_up: | | [src/wasm/ngx\_wasm\_core\_host.c](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571?src=pr&el=tree&filepath=src%2Fwasm%2Fngx_wasm_core_host.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong#diff-c3JjL3dhc20vbmd4X3dhc21fY29yZV9ob3N0LmM=) | `95.16129% <100.00000%> (+1.41128%)` | :arrow_up: | | [src/common/lua/ngx\_wasm\_lua.c](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571?src=pr&el=tree&filepath=src%2Fcommon%2Flua%2Fngx_wasm_lua.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong#diff-c3JjL2NvbW1vbi9sdWEvbmd4X3dhc21fbHVhLmM=) | `84.77612% <92.59259%> (+0.68521%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | [Flag](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | Coverage Δ | | |---|---|---| | [unit](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | `90.32999% <95.34884%> (+0.01551%)` | :arrow_up: | | [valgrind](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/571/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | `81.97727% <96.07843%> (+0.09020%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong#carryforward-flags-in-the-pull-request-comment) to find out more.