On 04/25/2014 03:38 AM, Amos Jeffries wrote: > On 25/04/2014 1:02 p.m., Alex Rousskov wrote: >> Hello, >> >> The attached patch Various at-exit-time leaks removed to minimize >> valgrind false positives. >> >> Removing these leaks helps with detecting true leaks, but it is possible >> that these changes expose other bugs that crash exiting Squid. We have >> not seen such crashes in our limited tests. >> >> >> Some of this cleanup code used to be enabled in LEAK_CHECK_MODE but then >> the whole cleanup code was marked as "broken" in 2006: >> >> #if LEAK_CHECK_MODE && 0 /* doesn't work at the moment */ >> >> It is possible that the parts of the cleanup code that this patch >> re-enables is not broken, but I do not really know that. >> >> >> Needless to say, at-exit leaks themselves are harmless because the OS is >> going to reclaim all process memory after the process exits. However, >> besides misleading valgrind, some (future?) cleanup code enabled by >> these changes might affect on-disk or shared memory state. >> >> These possible cleanup actions (that may be difficult to track reliably) >> make re-enabling LEAK_CHECK_MODE conditional for this code not such a >> good idea. Besides, if we do not enable the cleanup on a permanent >> basis, the usually unused code may never work correctly. >> >> Still, at-exit crashes are annoying and this patch might introduce them. >> >> >> I am not quite sure whether this patch should be committed, but will >> probably commit it if there are no other opinions.
> If these are going to be added to the main processing sequence they > should be Runners in the finishShutdown() hook please. I agree that using Runners would be better, but will not have the time to make such an improvement in the foreseeable future. If somebody has the time, please post a better patch. I will not commit this one. Thank you, Alex.
