Centrally destroy all explicit and implicit ACLs to avoid destruction segfaults during reconfiguration. Group ACLs created later may use other ACLs created earlier and vice versa, a group ACL created earlier may use other ACLs created later. The latter is possible when an ACL (e.g., A2 below) is declared when the group already exists: acl A1 src 127.0.0.1 acl Group all-of A1 acl A2 src 127.0.0.2 acl Group all-of A2 Thus, the group (i.e., InnerNode) ACL destructor may access already deleted children regardless of the global ACL deletion order (FIFO or LIFO with respect to ACL creation). Instead of relying on the deletion order to protect InnerNode, we remove the InnerNode ACL destructor completely and rely on a global set of registered ACLs to destroy all ACLs. The old code was destroying all explicit ACLs in the same centralized fashion. We now add implicit ACLs (commonly used by InnerNodes) to the centralized destruction sequence. We added a new destruction-dedicated container to avoid messing with the by-name ACL search that Config.aclList global is used for. This new container will become unnecessary once we start refcounting ACLs. === modified file 'src/acl/Acl.cc' --- src/acl/Acl.cc 2013-12-02 00:35:50 +0000 +++ src/acl/Acl.cc 2013-12-10 23:48:56 +0000 @@ -14,40 +14,41 @@ * sources; see the CREDITS file for full details. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or * (at your option) any later version. * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA. * */ #include "squid.h" #include "acl/Acl.h" #include "acl/Checklist.h" +#include "acl/Gadgets.h" #include "anyp/PortCfg.h" #include "cache_cf.h" #include "ConfigParser.h" #include "Debug.h" #include "dlink.h" #include "globals.h" #include "profiler/Profiler.h" #include "SquidConfig.h" const ACLFlag ACLFlags::NoFlags[1] = {ACL_F_END}; const char *AclMatchedName = NULL; bool ACLFlags::supported(const ACLFlag f) const { if (f == ACL_F_REGEX_CASE) return true; return (supported_.find(f) != std::string::npos); } @@ -278,46 +279,47 @@ ACL::ParseAclLine(ConfigParser &parser, /*split the function here */ A->parse(); /* * Clear AclMatchedName from our temporary hack */ AclMatchedName = NULL; /* ugly */ if (!new_acl) return; if (A->empty()) { debugs(28, DBG_CRITICAL, "Warning: empty ACL: " << A->cfgline); } if (!A->valid()) { fatalf("ERROR: Invalid ACL: %s\n", A->cfgline); } - // prepend so that ACLs declared later (and possibly using earlier ACLs) - // are destroyed earlier (before the ACLs they use are destroyed) + // add to the global list for searching explicit ACLs by name assert(head && *head == Config.aclList); - A->registered = true; A->next = *head; *head = A; + + // register for centralized cleanup + aclRegister(A); } bool ACL::isProxyAuth() const { return false; } /* ACL result caching routines */ int ACL::matchForCache(ACLChecklist *checklist) { /* This is a fatal to ensure that cacheMatchAcl calls are _only_ * made for supported acl types */ fatal("aclCacheMatchAcl: unknown or unexpected ACL type"); return 0; /* NOTREACHED */ } /* === modified file 'src/acl/Acl.h' --- src/acl/Acl.h 2013-12-02 00:36:24 +0000 +++ src/acl/Acl.h 2013-12-10 23:48:56 +0000 @@ -120,41 +120,41 @@ public: virtual ACL *clone()const = 0; /// parses node represenation in squid.conf; dies on failures virtual void parse() = 0; virtual char const *typeString() const = 0; virtual bool isProxyAuth() const; virtual wordlist *dump() const = 0; virtual bool empty () const = 0; virtual bool valid () const; int cacheMatchAcl(dlink_list * cache, ACLChecklist *); virtual int matchForCache(ACLChecklist *checklist); virtual void prepareForUse() {} char name[ACL_NAME_SZ]; char *cfgline; ACL *next; // XXX: remove or at least use refcounting ACLFlags flags; ///< The list of given ACL flags - bool registered; ///< added to Config.aclList and can be reused via by FindByName() + bool registered; ///< added to the global list of ACLs via aclRegister() public: class Prototype { public: Prototype (); Prototype (ACL const *, char const *); ~Prototype(); static bool Registered(char const *); static ACL *Factory (char const *); private: ACL const*prototype; char const *typeString; private: static Vector * Registry; static void *Initialized; === modified file 'src/acl/Gadgets.cc' --- src/acl/Gadgets.cc 2013-11-29 23:44:28 +0000 +++ src/acl/Gadgets.cc 2013-12-10 23:48:56 +0000 @@ -33,40 +33,48 @@ * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA. * */ #include "squid.h" #include "acl/Acl.h" #include "acl/AclDenyInfoList.h" #include "acl/AclNameList.h" #include "acl/Checklist.h" #include "acl/Gadgets.h" #include "acl/Strategised.h" #include "acl/Tree.h" #include "ConfigParser.h" #include "errorpage.h" #include "globals.h" #include "HttpRequest.h" #include "Mem.h" +#include +#include + + +typedef std::set AclSet; +/// Accumulates all ACLs to facilitate their clean deletion despite reuse. +static AclSet *RegisteredAcls; // TODO: Remove when ACLs are refcounted + /* does name lookup, returns page_id */ err_type aclGetDenyInfoPage(AclDenyInfoList ** head, const char *name, int redirect_allowed) { if (!name) { debugs(28, 3, "ERR_NONE due to a NULL name"); return ERR_NONE; } AclDenyInfoList *A = NULL; debugs(28, 8, HERE << "got called for " << name); for (A = *head; A; A = A->next) { AclNameList *L = NULL; if (!redirect_allowed && strchr(A->err_page_name, ':') ) { debugs(28, 8, HERE << "Skip '" << A->err_page_name << "' 30x redirects not allowed as response here."); continue; } @@ -227,57 +235,72 @@ aclParseAclList(ConfigParser &, Acl::Tre Acl::AndNode *rule = new Acl::AndNode; rule->context(ctxLine.content(), config_input_line); rule->lineParse(); MemBuf ctxTree; ctxTree.init(); ctxTree.Printf("%s %s", cfg_directive, label); ctxTree.terminate(); // We want a cbdata-protected Tree (despite giving it only one child node). 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; + } +} + /*********************/ /* Destroy functions */ /*********************/ +/// helper for RegisteredAcls cleanup +static void +aclDeleteOne(ACL *acl) +{ + delete acl; +} + +/// called to delete ALL Acls. void aclDestroyAcls(ACL ** head) { - ACL *next = NULL; - - debugs(28, 8, "aclDestroyACLs: invoked"); - - for (ACL *a = *head; a; a = next) { - next = a->next; - delete a; + *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(); } - - *head = NULL; } void aclDestroyAclList(ACLList **list) { debugs(28, 8, "aclDestroyAclList: invoked"); assert(list); cbdataFree(*list); } void aclDestroyAccessList(acl_access ** list) { assert(list); if (*list) debugs(28, 3, "destroying: " << *list << ' ' << (*list)->name); cbdataFree(*list); } /* maex@space.net (06.09.1996) === modified file 'src/acl/Gadgets.h' --- src/acl/Gadgets.h 2013-10-25 00:13:46 +0000 +++ src/acl/Gadgets.h 2013-12-10 23:48:56 +0000 @@ -1,35 +1,38 @@ #ifndef SQUID_ACL_GADGETS_H #define SQUID_ACL_GADGETS_H #include "acl/forward.h" #include "err_type.h" #if HAVE_SSTREAM #include #endif class ConfigParser; class dlink_list; class StoreEntry; class wordlist; +/// Register an ACL object for future deletion. Repeated registrations are OK. +/// \ingroup ACLAPI +void aclRegister(ACL *acl); /// \ingroup ACLAPI void aclDestroyAccessList(acl_access **list); /// \ingroup ACLAPI void aclDestroyAcls(ACL **); /// \ingroup ACLAPI void aclDestroyAclList(ACLList **); /// Parses a single line of a "action followed by acls" directive (e.g., http_access). /// \ingroup ACLAPI void aclParseAccessLine(const char *directive, ConfigParser &parser, Acl::Tree **); /// Parses a single line of a "some context followed by acls" directive (e.g., note n v). /// The label parameter identifies the context (for debugging). /// \ingroup ACLAPI void aclParseAclList(ConfigParser &parser, Acl::Tree **, const char *label); /// Template to convert various context lables to strings. \ingroup ACLAPI template inline void aclParseAclList(ConfigParser &parser, Acl::Tree **tree, const Any any) { std::ostringstream buf; buf << any; === modified file 'src/acl/InnerNode.cc' --- src/acl/InnerNode.cc 2013-05-28 22:33:15 +0000 +++ src/acl/InnerNode.cc 2013-12-10 23:48:56 +0000 @@ -1,48 +1,32 @@ #include "squid.h" #include "acl/Acl.h" #include "acl/BoolOps.h" #include "acl/Checklist.h" #include "acl/InnerNode.h" #include "cache_cf.h" #include "ConfigParser.h" #include "Debug.h" #include "globals.h" #include "wordlist.h" #include -// "delete acl" class to use with std::for_each() in InnerNode::~InnerNode() -class AclDeleter -{ -public: - void operator()(ACL* acl) { - // Do not delete explicit ACLs; they are maintained by Config.aclList. - if (acl && !acl->registered) - delete acl; - } -}; - -Acl::InnerNode::~InnerNode() -{ - std::for_each(nodes.begin(), nodes.end(), AclDeleter()); -} - 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); } // one call parses one "acl name acltype name1 name2 ..." line === modified file 'src/acl/InnerNode.h' --- src/acl/InnerNode.h 2013-12-02 00:36:24 +0000 +++ src/acl/InnerNode.h 2013-12-10 23:48:56 +0000 @@ -1,36 +1,36 @@ #ifndef SQUID_ACL_INNER_NODE_H #define SQUID_ACL_INNER_NODE_H #include "acl/Acl.h" #include namespace Acl { typedef std::vector Nodes; ///< a collection of nodes /// An intermediate ACL tree node. Manages a collection of child tree nodes. class InnerNode: public ACL { public: - virtual ~InnerNode(); + // No ~InnerNode() to delete children. They are aclRegister()ed instead. /// Resumes matching (suspended by an async call) at the given position. bool resumeMatchingAt(ACLChecklist *checklist, Acl::Nodes::const_iterator pos) const; /// the number of children nodes Nodes::size_type childrenCount() const { return nodes.size(); } /* ACL API */ virtual void prepareForUse(); virtual bool empty() const; virtual wordlist *dump() const; /// parses one "acl name type acl1 acl2..." line, appending to nodes void lineParse(); /// appends the node to the collection and takes control over it void add(ACL *node); protected: /// checks whether the nodes match, starting with the given one