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

On 06/14/2014 11:12 AM, Kinkie wrote:
> 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.

Committed as trunk r13469, with an explicit TODO.

Thank you,

Alex.

> 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.
>>>
>>
>
>
>
Received on Mon Jun 16 2014 - 22:54:47 MDT