> On 8 Mar 2024, at 19:45, Piotr Sikora via nginx-devel <nginx-devel@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 _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
