Re: [PATCH] At-exit leaks

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.
Received on Fri Apr 25 2014 - 16:02:48 MDT