Re: [PATCH] At-exit leaks

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.
>
>
> Thank you,
>
> Alex.
>

If these are going to be added to the main processing sequence they
should be Runners in the finishShutdown() hook please.

In trunk that will also ensure they are run before the pooled memory is
free'd.

Amos
Received on Fri Apr 25 2014 - 09:38:16 MDT