Re: [PATCH 1/8] reconfiguration leaks: implicit ACLs

I agree on the objective, and while this is not the solution it's a a
workaround; +1 but if you haven't already please add a TODO mentioning
the eventual refcount objective.

On Sat, Jun 14, 2014 at 3:21 AM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> On 14/06/2014 7:57 a.m., Alex Rousskov wrote:
>> On 04/25/2014 01:58 AM, Amos Jeffries wrote:
>>> On 25/04/2014 12:46 p.m., Alex Rousskov wrote:
>>>> Do not leak implicit ACLs during reconfigure.
>>>>
>>>> Many ACLs implicitly created by Squid when grouping multiple ACLs were
>>>> not destroyed because those implicit ACLs where not covered by the
>>>> global ACL registry used for ACL destruction.
>>>>
>>>> See also: r13210 which did not go far enough because it incorrectly
>>>> assumed that all InnerNode() children are aclRegister()ed and, hence,
>>>> will be centrally freed.
>>
>>
>>> -0.
>>
>> Is this a "negative" vote from "Squid3 voting" rules point of view?
>> http://wiki.squid-cache.org/MergeProcedure#Squid3_Voting
>
> It is "I don't like it but not objecting to a commit if you do it".
>
>
>>
>>
>>> I believe we should move to reference counting ACLs instead of
>>> continuing to work around these edge cases.
>>
>> I agree that reference counting is an overall better design for ACLs, of
>> course. However, since refcounting ACLs would be a large change that
>> nobody has volunteered to implement in the foreseeable future (AFAIK), I
>> suggest that this [significant] leak fix should go in now.
>>
>> Any other votes/opinions?
>>
>>
>> Thank you,
>>
>> Alex.
>>
>

-- 
    Francesco