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 <squ...@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
