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.

Also, do not use cbdataFree() on non-POD Acl::Tree objects that have
destructors.

TODO: Refcount all ACLs instead.

diff --git a/src/acl/Gadgets.cc b/src/acl/Gadgets.cc
index 4c104f249f52a56a9ca450f7398b05309ddd5511..e357a492b726eaef85bad7cddad4f5b180dd6ac0 100644 (file)
--- a/src/acl/Gadgets.cc
+++ b/src/acl/Gadgets.cc
@@ -262,17 +262,22 @@ aclRegister(ACL *acl)
     }
 }
 
+/// 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)
@@ -280,8 +285,15 @@ 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;
+        }
     }
 }
 
@@ -290,7 +302,8 @@ aclDestroyAclList(ACLList **list)
 {
     debugs(28, 8, "aclDestroyAclList: invoked");
     assert(list);
-    cbdataFree(*list);
+    delete *list;
+    *list = NULL;
 }
 
 void
@@ -299,7 +312,8 @@ aclDestroyAccessList(acl_access ** list)
     assert(list);
     if (*list)
         debugs(28, 3, "destroying: " << *list << ' ' << (*list)->name);
-    cbdataFree(*list);
+    delete *list;
+    *list = NULL;
 }
 
 /* maex@space.net (06.09.1996)

diff --git a/src/acl/InnerNode.cc b/src/acl/InnerNode.cc
index ca877d535c60c208243a9e7ce36c26453d897227..51a9bda68258c545921b48b07dfe48205c3e33d8 100644 (file)
--- a/src/acl/InnerNode.cc
+++ b/src/acl/InnerNode.cc
@@ -2,6 +2,7 @@
 #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"
@@ -26,6 +27,7 @@ Acl::InnerNode::add(ACL *node)
 {
     assert(node != NULL);
     nodes.push_back(node);
+    aclRegister(node);
 }
 
 // one call parses one "acl name acltype name1 name2 ..." line