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 1764be7f578a5e1f196aac4e91b76026677a9436..63e4469c2633c91e01201d5040d033a80c8d2d46 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 c8042ecc5e749f772f3c9158eb30ec1c33117f75..6dad6b3ad1ff5e17b0c8e5885405b6c55eed593e 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"
@@ -27,6 +28,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