On 12/10/2013 05:24 PM, Alex Rousskov wrote: > On 12/01/2013 05:43 PM, Alex Rousskov wrote: > >> The attached patch destroys ACLs in the reverse order of creation to >> avoid destruction segfaults during reconfiguration. > >> Done as trunk r13165. > > > Sorry, that was not enough. I somehow missed an obvious use case that > the committed fix does not cover, despite specifically trying to imagine > it during testing: It is actually possible for a group ACL G to use an > ACL A1 that was created before G and an ACL A2 that was created after G: > > acl A1 src 127.0.0.1 > acl G all-of A1 > acl A2 src 127.0.0.2 > acl G all-of A2 > > Such order of ACL creation makes any creation-based destruction order > invalid: Whether we use FIFO (as before) or LIFO (as after r13165), G's > destructor may dump core when deleting either A1 or A2 because one of > them will be already deleted earlier via Config.aclList. > > The attached patch fixes the problem differently. Instead of using > Config.aclList to register and destroy explicit ACLs and then relying on > InnerNode destructor to destroy its sometimes explicit children, the > take2 patch dedicates a new container to register and destroy both > implicit and explicit ACLs in one sweep. > > The InnerNode destructor is gone now, with aclDestroyAcls() replacing > its functionality. > > The Config.aclList global is still used for searching explicit ACLs by > name -- no changes there. > > Refcounting remains the preferred long-term solution, of course. I have > once again reviewed the changes required to make that happen, and can > confirm that they are too big for me to volunteer for that project right > now. Volunteers welcome.
Committed to trunk as r13210. Alex.
