[squid-dev] [PATCH] Squid crashes on shutdown while cleaning up idle ICAP connections Part2

[Christos Tsantilas]

Patch applied to trunk as r14825 with the requested changes.

On 09/07/2016 05:56 PM, Amos Jeffries wrote:
> On 7/09/2016 9:44 p.m., Christos Tsantilas wrote:
>> A preview of this patch originally discussed under the "[PATCH] Bug 4430
>> Squid crashes on shutdown while cleaning up idle ICAP connections" mail
>> thread on squid-dev:
>>   http://lists.squid-cache.org/pipermail/squid-dev/2016-March/005214.html
>>
>> We fixed the patch so I hope it handles most of the Amos objections for
>> the first patch.
>>
>> Patch description:
>>
>> The global Adaptation::Icap::TheConfig object is automatically destroyed
>> when Squid exits. Its destructor destroys Icap::ServiceRep objects that,
>> in turn, close all open connections in the idle connections pool. Since
>> this happens after comm_exit has destroyed all Comm structures
>> associated with those connections, Squid crashes.
>>
>> This patch uses updated RegisteredRunners API to close all connections
>> in existing connections pools before Squid cleans up the Comm subsystem.
>>
>> Also added a new IndependentRunner class to the RegistersRunner API,
>> which must be used for runners that are destroyed by external forces,
>> possibly while still being registered. IndependentRunner objects
>> unregister themselves from Runners registry when they are destroyed.
>>
>> The finishShutdown() method is now virtual and may be overloaded to
>> implement independent runner cleanup during main loop (and/or to support
>> complex cleanup actions that require calling other virtual methods). The
>> method is still called for all registered runners but it no longer
>> destroys the runner. For dependent runners, the destruction happens soon
>> after the finishShutdown event ends but also during the main loop
>> (unless the runner has managed to register after the main loop ended).
>>
>> This patch replaces the r14575 temporary fix.
>>
>> This is a Measurement Factory project.
>>
>
> I assume this has had enough testing since it last came up for audit.
>
> Please take the opportunity when touching code lines to remove HERE from
> debugs() lines, and also use nullptr instead of NULL.
>
> +1. Please apply ASAP with above changes.
>
> Amos
>
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
>