Hello,

    I am going to post several patches that fix various reconfiguration
leaks. The patches will be posted in no particular order. Here is a
Table of Contents:

  1. implicit ACLs,
  2. adaptation ACLs,
  3. TcpAcceptor job,
  4. Cache Manager menu items,
  5. [SSL] objects tied to http_port and https_port,
  6. fake SSL certificate context cache when reconfigure changes port,
  7. SSL ex_data for SSL state that survived reconfigure, a 12 y.o. bug!
  8. Excessive $HOME leak.


We fixed all reconfigure leaks we could reproduce and detect with
valgrind in one particular test environment, but we do not know whether
we fixed all the leaks. The production leaks that prompted these fixes
were huge (10s, perhaps 100s of MBs per reconfigure!), but we could not
reproduce leaks of similar magnitude. The key difference could be the
SSL certificates that Squid deals with, but we do not really know. More
work may be needed to make reconfiguration leak-free.

All of the patches were tested in production, albeit before they were
ported to the current trunk. At least one change (#5, the http_port
leak) may be considered controversial, but it also cures one of the
biggest leaks. Details are in the following emails.

The indentation in some of the patches was not fixed after adjusting the
surrounding code (on purpose, to show which code was preserved intact).
I hope I will not forget to fix that before committing any approved code.


I am posting the aggregate patch with this introductory email in case
you prefer to review all changes at once, but if you have any
change-specific comments or objections please respond to the
corresponding change-specific email instead of this one!

Those change-specific emails will have hand-edited portion of the
aggregate patch attached. I hope I got the hand-editing part right, but
you can consult the aggregate patch if anything is missing.


Thank you,

Alex.
Assorted fixes for reconfiguration leaks.

=== 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;
 }
 
 /* [email protected] (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);

=== modified file 'src/adaptation/AccessRule.cc'
--- src/adaptation/AccessRule.cc	2013-05-13 22:48:23 +0000
+++ src/adaptation/AccessRule.cc	2014-04-24 20:12:30 +0000
@@ -1,37 +1,38 @@
 #include "squid.h"
+#include "acl/Tree.h"
 #include "acl/Gadgets.h"
 #include "adaptation/AccessRule.h"
 #include "adaptation/Service.h"
 #include "adaptation/ServiceGroups.h"
 #include "ConfigParser.h"
 #include "Debug.h"
 
 int Adaptation::AccessRule::LastId = 0;
 
 Adaptation::AccessRule::AccessRule(const String &aGroupId): id(++LastId), groupId(aGroupId), acl(NULL)
 {
 }
 
 Adaptation::AccessRule::~AccessRule()
 {
-    // XXX: leaking acls here?
+    delete acl;
 }
 
 void
 Adaptation::AccessRule::parse(ConfigParser &parser)
 {
     aclParseAccessLine("adaptation_access", parser, &acl);
 }
 
 void
 Adaptation::AccessRule::finalize()
 {
     if (!group()) { // no explicit group
         debugs(93,7, HERE << "no service group: " << groupId);
         // try to add a one-service group
         if (FindService(groupId) != NULL) {
             ServiceGroupPointer g = new SingleService(groupId);
             g->finalize(); // explicit groups were finalized before rules
             AllGroups().push_back(g);
         }
     }

=== modified file 'src/anyp/PortCfg.cc'
--- src/anyp/PortCfg.cc	2014-04-16 21:58:08 +0000
+++ src/anyp/PortCfg.cc	2014-04-24 20:19:11 +0000
@@ -57,44 +57,46 @@ AnyP::PortCfg::PortCfg() :
         sslContextFlags(0),
         sslOptions(0)
 #endif
 {
     memset(&tcp_keepalive, 0, sizeof(tcp_keepalive));
 }
 
 AnyP::PortCfg::~PortCfg()
 {
     if (Comm::IsConnOpen(listenConn)) {
         listenConn->close();
         listenConn = NULL;
     }
 
     safe_free(name);
     safe_free(defaultsite);
 
 #if USE_OPENSSL
     safe_free(cert);
     safe_free(key);
-    safe_free(options);
     safe_free(cipher);
+    safe_free(options);
+    safe_free(clientca);
     safe_free(cafile);
     safe_free(capath);
+    safe_free(crlfile);
     safe_free(dhfile);
     safe_free(sslflags);
     safe_free(sslContextSessionId);
 #endif
 }
 
 AnyP::PortCfg *
 AnyP::PortCfg::clone() const
 {
     AnyP::PortCfg *b = new AnyP::PortCfg();
     b->s = s;
     if (name)
         b->name = xstrdup(name);
     if (defaultsite)
         b->defaultsite = xstrdup(defaultsite);
 
     b->transport = transport;
     b->flags = flags;
     b->allow_direct = allow_direct;
     b->vhost = vhost;

=== modified file 'src/cache_cf.cc'
--- src/cache_cf.cc	2014-04-12 17:32:34 +0000
+++ src/cache_cf.cc	2014-04-24 20:23:43 +0000
@@ -874,44 +874,49 @@ configDoConfigure(void)
                  * Andres Kroonmaa <[email protected]>:
                  * Some getpwnam() implementations (Solaris?) require
                  * an available FD < 256 for opening a FILE* to the
                  * passwd file.
                  * DW:
                  * This should be safe at startup, but might still fail
                  * during reconfigure.
                  */
                 fatalf("getpwnam failed to find userid for effective user '%s'",
                        Config.effectiveUser);
                 return;
             }
 
             Config2.effectiveUserID = pwd->pw_uid;
 
             Config2.effectiveGroupID = pwd->pw_gid;
 
 #if HAVE_PUTENV
 
             if (pwd->pw_dir && *pwd->pw_dir) {
+                // putenv() leaks by design; avoid leaks when nothing changes
+                static String lastDir;
+                if (!lastDir.size() || lastDir != pwd->pw_dir) {
+                    lastDir = pwd->pw_dir;
                 int len;
                 char *env_str = (char *)xcalloc((len = strlen(pwd->pw_dir) + 6), 1);
                 snprintf(env_str, len, "HOME=%s", pwd->pw_dir);
                 putenv(env_str);
+                }
             }
 
 #endif
 
         }
     } else {
         Config2.effectiveUserID = geteuid();
         Config2.effectiveGroupID = getegid();
     }
 
     if (NULL != Config.effectiveGroup) {
 
         struct group *grp = getgrnam(Config.effectiveGroup);
 
         if (NULL == grp) {
             fatalf("getgrnam failed to find groupid for effective group '%s'",
                    Config.effectiveGroup);
             return;
         }
 
@@ -3761,43 +3766,43 @@ parse_port_option(AnyP::PortCfg * s, cha
         s->generateHostCertificates = true;
     } else if (strcmp(token, "generate-host-certificates=off") == 0) {
         s->generateHostCertificates = false;
     } else if (strncmp(token, "dynamic_cert_mem_cache_size=", 28) == 0) {
         parseBytesOptionValue(&s->dynamicCertMemCacheSize, B_BYTES_STR, token + 28);
 #endif
     } else {
         debugs(3, DBG_CRITICAL, "FATAL: Unknown http(s)_port option '" << token << "'.");
         self_destruct();
     }
 }
 
 void
 add_http_port(char *portspec)
 {
     AnyP::PortCfg *s = new AnyP::PortCfg();
     s->setTransport("HTTP");
     parsePortSpecification(s, portspec);
     // we may need to merge better if the above returns a list with clones
     assert(s->next == NULL);
-    s->next = cbdataReference(Config.Sockaddr.http);
-    cbdataReferenceDone(Config.Sockaddr.http);
-    Config.Sockaddr.http = cbdataReference(s);
+    // no cbdata locking here: Config owns the ports (deleted in free_PortCfg)
+    s->next = Config.Sockaddr.http;
+    Config.Sockaddr.http = s;
 }
 
 static void
 parsePortCfg(AnyP::PortCfg ** head, const char *optionName)
 {
     const char *protocol = NULL;
     if (strcmp(optionName, "http_port") == 0 ||
             strcmp(optionName, "ascii_port") == 0)
         protocol = "http";
     else if (strcmp(optionName, "https_port") == 0)
         protocol = "https";
     if (!protocol) {
         self_destruct();
         return;
     }
 
     char *token = ConfigParser::NextToken();
 
     if (!token) {
         self_destruct();
@@ -3820,42 +3825,42 @@ parsePortCfg(AnyP::PortCfg ** head, cons
         if (s->flags.tunnelSslBumping && !hijacked) {
             debugs(3, DBG_CRITICAL, "FATAL: ssl-bump on https_port requires tproxy/intercept which is missing.");
             self_destruct();
         }
         if (hijacked && !s->flags.tunnelSslBumping) {
             debugs(3, DBG_CRITICAL, "FATAL: tproxy/intercept on https_port requires ssl-bump which is missing.");
             self_destruct();
         }
     }
 #endif
 
     if (Ip::EnableIpv6&IPV6_SPECIAL_SPLITSTACK && s->s.isAnyAddr()) {
         // clone the port options from *s to *(s->next)
         s->next = cbdataReference(s->clone());
         s->next->s.setIPv4();
         debugs(3, 3, AnyP::UriScheme(s->transport.protocol).c_str() << "_port: clone wildcard address for split-stack: " << s->s << " and " << s->next->s);
     }
 
     while (*head)
         head = &(*head)->next;
-
-    *head = cbdataReference(s);
+    // no cbdata locking here: Config owns the ports (deleted in free_PortCfg)
+    *head = s;
 }
 
 static void
 dump_generic_port(StoreEntry * e, const char *n, const AnyP::PortCfg * s)
 {
     char buf[MAX_IPSTRLEN];
 
     storeAppendPrintf(e, "%s %s",
                       n,
                       s->s.toUrl(buf,MAX_IPSTRLEN));
 
     // MODES and specific sub-options.
     if (s->flags.natIntercept)
         storeAppendPrintf(e, " intercept");
 
     else if (s->flags.tproxyIntercept)
         storeAppendPrintf(e, " tproxy");
 
     else if (s->flags.accelSurrogate) {
         storeAppendPrintf(e, " accel");
@@ -3965,50 +3970,51 @@ dump_generic_port(StoreEntry * e, const 
 #endif
 }
 
 static void
 dump_PortCfg(StoreEntry * e, const char *n, const AnyP::PortCfg * s)
 {
     while (s) {
         dump_generic_port(e, n, s);
         storeAppendPrintf(e, "\n");
         s = s->next;
     }
 }
 
 static void
 free_PortCfg(AnyP::PortCfg ** head)
 {
     AnyP::PortCfg *s;
 
     while ((s = *head) != NULL) {
         *head = s->next;
-        cbdataReferenceDone(s);
+        delete s;
     }
 }
 
 void
 configFreeMemory(void)
 {
     free_all();
 #if USE_OPENSSL
     SSL_CTX_free(Config.ssl_client.sslContext);
+    Config.ssl_client.sslContext = NULL;
 #endif
 }
 
 void
 requirePathnameExists(const char *name, const char *path)
 {
 
     struct stat sb;
     char pathbuf[BUFSIZ];
     assert(path != NULL);
 
     if (Config.chroot_dir && (geteuid() == 0)) {
         snprintf(pathbuf, BUFSIZ, "%s/%s", Config.chroot_dir, path);
         path = pathbuf;
     }
 
     if (stat(path, &sb) < 0) {
         debugs(0, DBG_CRITICAL, (opt_parse_cfg_only?"FATAL ":"") << "ERROR: " << name << " " << path << ": " << xstrerror());
         // keep going to find more issues if we are only checking the config file with "-k parse"
         if (opt_parse_cfg_only)

=== modified file 'src/cache_manager.cc'
--- src/cache_manager.cc	2013-10-25 00:13:46 +0000
+++ src/cache_manager.cc	2014-04-24 20:12:30 +0000
@@ -65,41 +65,41 @@ class ClassActionCreator: public Mgr::Ac
 {
 public:
     typedef Mgr::Action::Pointer Handler(const Mgr::Command::Pointer &cmd);
 
 public:
     ClassActionCreator(Handler *aHandler): handler(aHandler) {}
 
     virtual Mgr::Action::Pointer create(const Mgr::Command::Pointer &cmd) const {
         return handler(cmd);
     }
 
 private:
     Handler *handler;
 };
 
 /// Registers new profiles, ignoring attempts to register a duplicate
 void
 CacheManager::registerProfile(const Mgr::ActionProfile::Pointer &profile)
 {
     Must(profile != NULL);
-    if (std::find(menu_.begin(), menu_.end(), profile) == menu_.end()) {
+    if (!CacheManager::findAction(profile->name)) {
         menu_.push_back(profile);
         debugs(16, 3, HERE << "registered profile: " << *profile);
     } else {
         debugs(16, 2, HERE << "skipped duplicate profile: " << *profile);
     }
 }
 
 /**
  \ingroup CacheManagerAPI
  * Registers a C-style action, which is implemented as a pointer to a function
  * taking as argument a pointer to a StoreEntry and returning void.
  * Implemented via CacheManagerActionLegacy.
  */
 void
 CacheManager::registerProfile(char const * action, char const * desc, OBJH * handler, int pw_req_flag, int atomic)
 {
     debugs(16, 3, HERE << "registering legacy " << action);
     const Mgr::ActionProfile::Pointer profile = new Mgr::ActionProfile(action,
             desc, pw_req_flag, atomic, new Mgr::FunActionCreator(handler));
     registerProfile(profile);

=== modified file 'src/client_side.cc'
--- src/client_side.cc	2014-03-31 06:57:27 +0000
+++ src/client_side.cc	2014-04-24 20:12:30 +0000
@@ -3704,43 +3704,48 @@ ConnStateData::sslCrtdHandleReplyWrapper
     ConnStateData * state_data = (ConnStateData *)(data);
     state_data->sslCrtdHandleReply(reply);
 }
 
 void
 ConnStateData::sslCrtdHandleReply(const HelperReply &reply)
 {
     if (reply.result == HelperReply::BrokenHelper) {
         debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " cannot be generated. ssl_crtd response: " << reply);
     } else if (!reply.other().hasContent()) {
         debugs(1, DBG_IMPORTANT, HERE << "\"ssl_crtd\" helper returned <NULL> reply.");
     } else {
         Ssl::CrtdMessage reply_message(Ssl::CrtdMessage::REPLY);
         if (reply_message.parse(reply.other().content(), reply.other().contentSize()) != Ssl::CrtdMessage::OK) {
             debugs(33, 5, HERE << "Reply from ssl_crtd for " << sslConnectHostOrIp << " is incorrect");
         } else {
             if (reply.result != HelperReply::Okay) {
                 debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " cannot be generated. ssl_crtd response: " << reply_message.getBody());
             } else {
                 debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " was successfully recieved from ssl_crtd");
-                SSL_CTX *ctx = Ssl::generateSslContextUsingPkeyAndCertFromMemory(reply_message.getBody().c_str(), *port);
-                getSslContextDone(ctx, true);
-                return;
+                if (cbdataReferenceValid(port)) {
+                    SSL_CTX *ctx = Ssl::generateSslContextUsingPkeyAndCertFromMemory(reply_message.getBody().c_str(), *port);
+                    getSslContextDone(ctx, true);
+                    return;
+                } else {
+                    debugs(33, 3, "but our port is gone");
+                    // getSslContextDone() will cleanup
+                }
             }
         }
     }
     getSslContextDone(NULL);
 }
 
 void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &certProperties)
 {
     certProperties.commonName =  sslCommonName.size() > 0 ? sslCommonName.termedBuf() : sslConnectHostOrIp.termedBuf();
 
     // fake certificate adaptation requires bump-server-first mode
     if (!sslServerBump) {
         assert(port->signingCert.get());
         certProperties.signWithX509.resetAndLock(port->signingCert.get());
         if (port->signPkey.get())
             certProperties.signWithPkey.resetAndLock(port->signPkey.get());
         certProperties.signAlgorithm = Ssl::algSignTrusted;
         return;
     }
 
@@ -3806,41 +3811,41 @@ void ConnStateData::buildSslCertGenerati
         assert(port->untrustedSigningCert.get());
         certProperties.signWithX509.resetAndLock(port->untrustedSigningCert.get());
         certProperties.signWithPkey.resetAndLock(port->untrustedSignPkey.get());
     } else {
         assert(port->signingCert.get());
         certProperties.signWithX509.resetAndLock(port->signingCert.get());
 
         if (port->signPkey.get())
             certProperties.signWithPkey.resetAndLock(port->signPkey.get());
     }
     signAlgorithm = certProperties.signAlgorithm;
 }
 
 void
 ConnStateData::getSslContextStart()
 {
     assert(areAllContextsForThisConnection());
     freeAllContexts();
     /* careful: freeAllContexts() above frees request, host, etc. */
 
-    if (port->generateHostCertificates) {
+    if (cbdataReferenceValid(port) && port->generateHostCertificates) {
         Ssl::CertificateProperties certProperties;
         buildSslCertGenerationParams(certProperties);
         sslBumpCertKey = certProperties.dbKey().c_str();
         assert(sslBumpCertKey.size() > 0 && sslBumpCertKey[0] != '\0');
 
         debugs(33, 5, HERE << "Finding SSL certificate for " << sslBumpCertKey << " in cache");
         Ssl::LocalContextStorage *ssl_ctx_cache = Ssl::TheGlobalContextStorage.getLocalStorage(port->s);
         SSL_CTX * dynCtx = NULL;
         Ssl::SSL_CTX_Pointer *cachedCtx = ssl_ctx_cache ? ssl_ctx_cache->get(sslBumpCertKey.termedBuf()) : NULL;
         if (cachedCtx && (dynCtx = cachedCtx->get())) {
             debugs(33, 5, HERE << "SSL certificate for " << sslBumpCertKey << " have found in cache");
             if (Ssl::verifySslCertificate(dynCtx, certProperties)) {
                 debugs(33, 5, HERE << "Cached SSL certificate for " << sslBumpCertKey << " is valid");
                 getSslContextDone(dynCtx);
                 return;
             } else {
                 debugs(33, 5, HERE << "Cached SSL certificate for " << sslBumpCertKey << " is out of date. Delete this certificate from cache");
                 if (ssl_ctx_cache)
                     ssl_ctx_cache->del(sslBumpCertKey.termedBuf());
             }
@@ -3860,40 +3865,48 @@ ConnStateData::getSslContextStart()
         } catch (const std::exception &e) {
             debugs(33, DBG_IMPORTANT, "ERROR: Failed to compose ssl_crtd " <<
                    "request for " << certProperties.commonName <<
                    " certificate: " << e.what() << "; will now block to " <<
                    "generate that certificate.");
             // fall through to do blocking in-process generation.
         }
 #endif // USE_SSL_CRTD
 
         debugs(33, 5, HERE << "Generating SSL certificate for " << certProperties.commonName);
         dynCtx = Ssl::generateSslContext(certProperties, *port);
         getSslContextDone(dynCtx, true);
         return;
     }
     getSslContextDone(NULL);
 }
 
 void
 ConnStateData::getSslContextDone(SSL_CTX * sslContext, bool isNew)
 {
+    if (!cbdataReferenceValid(port)) { // reconfigure may delete our port
+        debugs(83, 2, "Closing SSL " << clientConnection->remote << " for lost port");
+        if (sslContext && Comm::IsConnOpen(clientConnection))
+            fd_table[clientConnection->fd].dynamicSslContext = sslContext;
+        clientConnection->close();
+        return;
+    }
+
     // Try to add generated ssl context to storage.
     if (port->generateHostCertificates && isNew) {
 
         if (signAlgorithm == Ssl::algSignTrusted) {
             // Add signing certificate to the certificates chain
             X509 *cert = port->signingCert.get();
             if (SSL_CTX_add_extra_chain_cert(sslContext, cert)) {
                 // increase the certificate lock
                 CRYPTO_add(&(cert->references),1,CRYPTO_LOCK_X509);
             } else {
                 const int ssl_error = ERR_get_error();
                 debugs(33, DBG_IMPORTANT, "WARNING: can not add signing certificate to SSL context chain: " << ERR_error_string(ssl_error, NULL));
             }
             Ssl::addChainToSslContext(sslContext, port->certsToChain.get());
         }
         //else it is self-signed or untrusted do not attrach any certificate
 
         Ssl::LocalContextStorage *ssl_ctx_cache = Ssl::TheGlobalContextStorage.getLocalStorage(port->s);
         assert(sslBumpCertKey.size() > 0 && sslBumpCertKey[0] != '\0');
         if (sslContext) {

=== modified file 'src/comm/TcpAcceptor.cc'
--- src/comm/TcpAcceptor.cc	2014-04-14 06:50:31 +0000
+++ src/comm/TcpAcceptor.cc	2014-04-24 20:12:30 +0000
@@ -105,40 +105,46 @@ Comm::TcpAcceptor::doneAll() const
 {
     // stop when FD is closed
     if (!IsConnOpen(conn)) {
         return AsyncJob::doneAll();
     }
 
     // stop when handlers are gone
     if (theCallSub == NULL) {
         return AsyncJob::doneAll();
     }
 
     // open FD with handlers...keep accepting.
     return false;
 }
 
 void
 Comm::TcpAcceptor::swanSong()
 {
     debugs(5,5, HERE);
     unsubscribe("swanSong");
+    if (IsConnOpen(conn)) {
+        if (closer_ != NULL)
+            comm_remove_close_handler(conn->fd, closer_);
+        conn->close();
+    }
+
     conn = NULL;
     AcceptLimiter::Instance().removeDead(this);
     AsyncJob::swanSong();
 }
 
 const char *
 Comm::TcpAcceptor::status() const
 {
     if (conn == NULL)
         return "[nil connection]";
 
     static char ipbuf[MAX_IPSTRLEN] = {'\0'};
     if (ipbuf[0] == '\0')
         conn->local.toHostStr(ipbuf, MAX_IPSTRLEN);
 
     static MemBuf buf;
     buf.reset();
     buf.Printf(" FD %d, %s",conn->fd, ipbuf);
 
     const char *jobStatus = AsyncJob::status();
@@ -165,42 +171,57 @@ Comm::TcpAcceptor::setListen()
     }
 
     if (Config.accept_filter && strcmp(Config.accept_filter, "none") != 0) {
 #ifdef SO_ACCEPTFILTER
         struct accept_filter_arg afa;
         bzero(&afa, sizeof(afa));
         debugs(5, DBG_IMPORTANT, "Installing accept filter '" << Config.accept_filter << "' on " << conn);
         xstrncpy(afa.af_name, Config.accept_filter, sizeof(afa.af_name));
         if (setsockopt(conn->fd, SOL_SOCKET, SO_ACCEPTFILTER, &afa, sizeof(afa)) < 0)
             debugs(5, DBG_CRITICAL, "WARNING: SO_ACCEPTFILTER '" << Config.accept_filter << "': '" << xstrerror());
 #elif defined(TCP_DEFER_ACCEPT)
         int seconds = 30;
         if (strncmp(Config.accept_filter, "data=", 5) == 0)
             seconds = atoi(Config.accept_filter + 5);
         if (setsockopt(conn->fd, IPPROTO_TCP, TCP_DEFER_ACCEPT, &seconds, sizeof(seconds)) < 0)
             debugs(5, DBG_CRITICAL, "WARNING: TCP_DEFER_ACCEPT '" << Config.accept_filter << "': '" << xstrerror());
 #else
         debugs(5, DBG_CRITICAL, "WARNING: accept_filter not supported on your OS");
 #endif
     }
+
+    typedef CommCbMemFunT<Comm::TcpAcceptor, CommCloseCbParams> Dialer;
+    closer_ = JobCallback(5, 4, Dialer, this, Comm::TcpAcceptor::handleClosure);
+    comm_add_close_handler(conn->fd, closer_);
 }
 
+/// called when listening descriptor is closed by an external force
+/// such as clientHttpConnectionsClose()
+void
+Comm::TcpAcceptor::handleClosure(const CommCloseCbParams &io)
+{
+    closer_ = NULL;
+    conn = NULL;
+    Must(done());
+}
+
+
 /**
  * This private callback is called whenever a filedescriptor is ready
  * to dupe itself and fob off an accept()ed connection
  *
  * It will either do that accept operation. Or if there are not enough FD
  * available to do the clone safely will push the listening FD into a list
  * of deferred operations. The list gets kicked and the dupe/accept() actually
  * done later when enough sockets become available.
  */
 void
 Comm::TcpAcceptor::doAccept(int fd, void *data)
 {
     try {
         debugs(5, 2, HERE << "New connection on FD " << fd);
 
         Must(isOpen(fd));
         TcpAcceptor *afd = static_cast<TcpAcceptor*>(data);
 
         if (!okToAccept()) {
             AcceptLimiter::Instance().defer(afd);

=== modified file 'src/comm/TcpAcceptor.h'
--- src/comm/TcpAcceptor.h	2013-10-25 00:13:46 +0000
+++ src/comm/TcpAcceptor.h	2014-04-24 20:12:30 +0000
@@ -1,29 +1,31 @@
 #ifndef SQUID_COMM_TCPACCEPTOR_H
 #define SQUID_COMM_TCPACCEPTOR_H
 
 #include "base/AsyncJob.h"
 #include "base/CbcPointer.h"
 #include "base/Subscription.h"
 #include "comm/forward.h"
 #include "comm_err_t.h"
 
+class CommCloseCbParams;
+
 namespace Comm
 {
 
 class AcceptLimiter;
 
 /**
  * Listens on a Comm::Connection for new incoming connections and
  * emits an active Comm::Connection descriptor for the new client.
  *
  * Handles all event limiting required to quash inbound connection
  * floods within the global FD limits of available Squid_MaxFD and
  * client_ip_max_connections.
  *
  * Fills the emitted connection with all connection details able to
  * be looked up. Currently these are the local/remote IP:port details
  * and the listening socket transparent-mode flag.
  */
 class TcpAcceptor : public AsyncJob
 {
 public:
@@ -56,37 +58,41 @@ public:
      */
     void acceptNext();
 
     /// Call the subscribed callback handler with details about a new connection.
     void notify(const comm_err_t flag, const Comm::ConnectionPointer &details) const;
 
     /// errno code of the last accept() or listen() action if one occurred.
     int errcode;
 
 protected:
     friend class AcceptLimiter;
     int32_t isLimited;                   ///< whether this socket is delayed and on the AcceptLimiter queue.
 
 private:
     Subscription::Pointer theCallSub;    ///< used to generate AsyncCalls handling our events.
 
     /// conn being listened on for new connections
     /// Reserved for read-only use.
     ConnectionPointer conn;
 
+    AsyncCall::Pointer closer_; ///< listen socket closure handler
+
     /// Method to test if there are enough file descriptors to open a new client connection
     /// if not the accept() will be postponed
     static bool okToAccept();
 
     /// Method callback for whenever an FD is ready to accept a client connection.
     static void doAccept(int fd, void *data);
 
     void acceptOne();
     comm_err_t oldAccept(Comm::ConnectionPointer &details);
     void setListen();
 
+    void handleClosure(const CommCloseCbParams &io);
+
     CBDATA_CLASS2(TcpAcceptor);
 };
 
 } // namespace Comm
 
 #endif /* SQUID_COMM_TCPACCEPTOR_H */

=== modified file 'src/ssl/context_storage.cc'
--- src/ssl/context_storage.cc	2014-03-30 12:00:34 +0000
+++ src/ssl/context_storage.cc	2014-04-24 20:35:45 +0000
@@ -73,36 +73,37 @@ Ssl::LocalContextStorage *Ssl::GlobalCon
         return NULL;
     else
         return i->second;
 }
 
 void Ssl::GlobalContextStorage::reconfigureStart()
 {
     configureStorage.clear();
     reconfiguring = true;
 }
 
 void Ssl::GlobalContextStorage::reconfigureFinish()
 {
     if (reconfiguring) {
         reconfiguring = false;
 
         // remove or change old local storages.
         for (std::map<Ip::Address, LocalContextStorage *>::iterator i = storage.begin(); i != storage.end(); ++i) {
             std::map<Ip::Address, size_t>::iterator conf_i = configureStorage.find(i->first);
             if (conf_i == configureStorage.end() || conf_i->second <= 0) {
+                delete i->second;
                 storage.erase(i);
             } else {
                 i->second->setMemLimit(conf_i->second);
             }
         }
 
         // add new local storages.
         for (std::map<Ip::Address, size_t>::iterator conf_i = configureStorage.begin(); conf_i != configureStorage.end(); ++conf_i ) {
             if (storage.find(conf_i->first) == storage.end() && conf_i->second > 0) {
                 storage.insert(std::pair<Ip::Address, LocalContextStorage *>(conf_i->first, new LocalContextStorage(-1, conf_i->second)));
             }
         }
     }
 }
 
 Ssl::GlobalContextStorage Ssl::TheGlobalContextStorage;

=== modified file 'src/ssl/support.cc'
--- src/ssl/support.cc	2014-03-30 12:00:34 +0000
+++ src/ssl/support.cc	2014-04-24 20:37:38 +0000
@@ -694,69 +694,69 @@ static void
 ssl_free_CertChain(void *, void *ptr, CRYPTO_EX_DATA *,
                    int, long, void *)
 {
     STACK_OF(X509) *certsChain = static_cast <STACK_OF(X509) *>(ptr);
     sk_X509_pop_free(certsChain,X509_free);
 }
 
 // "free" function for X509 certificates
 static void
 ssl_free_X509(void *, void *ptr, CRYPTO_EX_DATA *,
               int, long, void *)
 {
     X509  *cert = static_cast <X509 *>(ptr);
     X509_free(cert);
 }
 
 /// \ingroup ServerProtocolSSLInternal
 static void
 ssl_initialize(void)
 {
-    static int ssl_initialized = 0;
+    static bool initialized = false;
+    if (initialized)
+        return;
+    initialized = true;
 
-    if (!ssl_initialized) {
-        ssl_initialized = 1;
         SSL_load_error_strings();
         SSLeay_add_ssl_algorithms();
 #if HAVE_OPENSSL_ENGINE_H
 
         if (Config.SSL.ssl_engine) {
             ENGINE *e;
 
             if (!(e = ENGINE_by_id(Config.SSL.ssl_engine))) {
                 fatalf("Unable to find SSL engine '%s'\n", Config.SSL.ssl_engine);
             }
 
             if (!ENGINE_set_default(e, ENGINE_METHOD_ALL)) {
                 int ssl_error = ERR_get_error();
                 fatalf("Failed to initialise SSL engine: %s\n",
                        ERR_error_string(ssl_error, NULL));
             }
         }
 
 #else
         if (Config.SSL.ssl_engine) {
             fatalf("Your OpenSSL has no SSL engine support\n");
         }
 
 #endif
-    }
 
     ssl_ex_index_server = SSL_get_ex_new_index(0, (void *) "server", NULL, NULL, NULL);
     ssl_ctx_ex_index_dont_verify_domain = SSL_CTX_get_ex_new_index(0, (void *) "dont_verify_domain", NULL, NULL, NULL);
     ssl_ex_index_cert_error_check = SSL_get_ex_new_index(0, (void *) "cert_error_check", NULL, &ssl_dupAclChecklist, &ssl_freeAclChecklist);
     ssl_ex_index_ssl_error_detail = SSL_get_ex_new_index(0, (void *) "ssl_error_detail", NULL, NULL, &ssl_free_ErrorDetail);
     ssl_ex_index_ssl_peeked_cert  = SSL_get_ex_new_index(0, (void *) "ssl_peeked_cert", NULL, NULL, &ssl_free_X509);
     ssl_ex_index_ssl_errors =  SSL_get_ex_new_index(0, (void *) "ssl_errors", NULL, NULL, &ssl_free_SslErrors);
     ssl_ex_index_ssl_cert_chain = SSL_get_ex_new_index(0, (void *) "ssl_cert_chain", NULL, NULL, &ssl_free_CertChain);
     ssl_ex_index_ssl_validation_counter = SSL_get_ex_new_index(0, (void *) "ssl_validation_counter", NULL, NULL, &ssl_free_int);
 }
 
 /// \ingroup ServerProtocolSSLInternal
 static int
 ssl_load_crl(SSL_CTX *sslContext, const char *CRLfile)
 {
     X509_STORE *st = SSL_CTX_get_cert_store(sslContext);
     X509_CRL *crl;
     BIO *in = BIO_new_file(CRLfile, "r");
     int count = 0;
 

Reply via email to