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.
Alex.
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.
=== modified file 'src/acl/Gadgets.cc'
--- src/acl/Gadgets.cc 2014-02-08 13:36:42 +0000
+++ src/acl/Gadgets.cc 2014-04-24 20:12:30 +0000
@@ -245,78 +245,92 @@ aclParseAclList(ConfigParser &, Acl::Tre
Acl::Tree *tree = new Acl::Tree;
tree->add(rule);
tree->context(ctxTree.content(), config_input_line);
assert(treep);
assert(!*treep);
*treep = tree;
}
void
aclRegister(ACL *acl)
{
if (!acl->registered) {
if (!RegisteredAcls)
RegisteredAcls = new AclSet;
RegisteredAcls->insert(acl);
acl->registered = true;
}
}
+/// remove registered acl from the centralized deletion set
+static
+void
+aclDeregister(ACL *acl)
+{
+ if (acl->registered) {
+ if (RegisteredAcls)
+ RegisteredAcls->erase(acl);
+ acl->registered = false;
+ }
+}
+
/*********************/
/* Destroy functions */
/*********************/
-/// helper for RegisteredAcls cleanup
-static void
-aclDeleteOne(ACL *acl)
-{
- delete acl;
-}
-
/// called to delete ALL Acls.
void
aclDestroyAcls(ACL ** head)
{
*head = NULL; // Config.aclList
if (AclSet *acls = RegisteredAcls) {
debugs(28, 8, "deleting all " << acls->size() << " ACLs");
- std::for_each(acls->begin(), acls->end(), &aclDeleteOne);
- acls->clear();
+ while (!acls->empty()) {
+ ACL *acl = *acls->begin();
+ // We use centralized deletion (this function) so ~ACL should not
+ // delete other ACLs, but we still deregister first to prevent any
+ // accesses to the being-deleted ACL via RegisteredAcls.
+ assert(acl->registered); // make sure we are making progress
+ aclDeregister(acl);
+ delete acl;
+ }
}
}
void
aclDestroyAclList(ACLList **list)
{
debugs(28, 8, "aclDestroyAclList: invoked");
assert(list);
- cbdataFree(*list);
+ delete *list;
+ *list = NULL;
}
void
aclDestroyAccessList(acl_access ** list)
{
assert(list);
if (*list)
debugs(28, 3, "destroying: " << *list << ' ' << (*list)->name);
- cbdataFree(*list);
+ delete *list;
+ *list = NULL;
}
/* m...@space.net (06.09.1996)
* destroy an AclDenyInfoList */
void
aclDestroyDenyInfoList(AclDenyInfoList ** list)
{
AclDenyInfoList *a = NULL;
AclDenyInfoList *a_next = NULL;
AclNameList *l = NULL;
AclNameList *l_next = NULL;
debugs(28, 8, "aclDestroyDenyInfoList: invoked");
for (a = *list; a; a = a_next) {
for (l = a->acl_list; l; l = l_next) {
l_next = l->next;
safe_free(l);
}
=== modified file 'src/acl/InnerNode.cc'
--- src/acl/InnerNode.cc 2014-04-12 07:10:39 +0000
+++ src/acl/InnerNode.cc 2014-04-24 20:14:09 +0000
@@ -1,48 +1,50 @@
#include "squid.h"
#include "acl/Acl.h"
#include "acl/BoolOps.h"
#include "acl/Checklist.h"
+#include "acl/Gadgets.h"
#include "acl/InnerNode.h"
#include "cache_cf.h"
#include "ConfigParser.h"
#include "Debug.h"
#include "globals.h"
#include <algorithm>
void
Acl::InnerNode::prepareForUse()
{
std::for_each(nodes.begin(), nodes.end(), std::mem_fun(&ACL::prepareForUse));
}
bool
Acl::InnerNode::empty() const
{
return nodes.empty();
}
void
Acl::InnerNode::add(ACL *node)
{
assert(node != NULL);
nodes.push_back(node);
+ aclRegister(node);
}
// one call parses one "acl name acltype name1 name2 ..." line
// kids use this method to handle [multiple] parse() calls correctly
void
Acl::InnerNode::lineParse()
{
// XXX: not precise, may change when looping or parsing multiple lines
if (!cfgline)
cfgline = xstrdup(config_input_line);
// expect a list of ACL names, each possibly preceeded by '!' for negation
while (const char *t = ConfigParser::strtokFile()) {
const bool negated = (*t == '!');
if (negated)
++t;
debugs(28, 3, "looking for ACL " << t);
ACL *a = ACL::FindByName(t);
