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

[Amos Jeffries]

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