[PATCH] Core: free connections and read/write events at shutdown

[Sergey Kandaurov]

> On 8 Mar 2024, at 19:45, Piotr Sikora via nginx-devel <nginx-devel at nginx.org> wrote:
> 
> Hi Sergey,
> 
>> Since this is not a real memory leak (the allocations made in the
>> init_process method of the ngx_event_core_module module are used up
>> to this function and already freed on exit automatically, as this
>> function never returns), I don't think there is something to fix.
> 
> Agreed, this is not a "real" leak.
> 
> But fixing those "false" leaks allows you to run test with sanitizers
> on the CI, which can protect you and/or external contributors from
> introducing new bugs.

While I agree that false positives do not allow to run LeakSanitizer
in a clean fashion, I don't think it is nginx which should be fixed.
Rather, sanitizer analysis could be improved instead to prevent FPs.

Meanwhile, leak sanitizer can be used with suppressions as appropriate
to run tests cleanly.  For example, this allows to suppress memory leak
reports for allocations made during worker process init, such as cycle
connections and read/write events:

$ cat suppr.txt
leak:ngx_worker_process_init
$ LSAN_OPTIONS=suppressions=suppr.txt prove -r

> 
>> (Further, the patch misses cycle->files for NGX_USE_FD_EVENT event
>> methods.
> 
> Good catch, thanks!
> 
>> Also, it's probably better to free the memory in the
>> exit_process method to obey the ownership (this would also fix
>> missing ngx_free() calls on win32), but this would require moving
>> code that uses these connections afterwards to catch socket leaks.)
> 
> Freeing memory in exit_process could result in use-after-free, since
> cleanups for the cycle->pool might still access those connections.
> 

Yep, that's why it is freed afterwards.

-- 
Sergey Kandaurov