From: Amos Jeffries <squid3@treenet.co.nz>
Date: Sat, 4 Jun 2011 12:48:45 +0000 (+1200)
Subject: Sync with trunk
X-Git-Tag: take08~55^2~124^2~6
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=983983ce5ef0d6785a1f6a65b1685e07ea68a9af;p=thirdparty%2Fsquid.git

Sync with trunk
---

983983ce5ef0d6785a1f6a65b1685e07ea68a9af
diff --cc src/HttpRequest.cc
index f4ce330caf,580938159b..c73af9c6c0
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@@ -226,6 -229,45 +226,43 @@@ HttpRequest::clone() cons
      return copy;
  }
  
+ bool
+ HttpRequest::inheritProperties(const HttpMsg *aMsg)
+ {
+     const HttpRequest* aReq = dynamic_cast<const HttpRequest*>(aMsg);
+     if (!aReq)
+         return false;
+ 
+     client_addr = aReq->client_addr;
+ #if FOLLOW_X_FORWARDED_FOR
+     indirect_client_addr = aReq->indirect_client_addr;
 -#endif
 -#if USE_SQUID_EUI
 -    client_eui48 = aReq->client_eui48;
 -    client_eui64 = aReq->client_eui64;
+ #endif
+     my_addr = aReq->my_addr;
+ 
+     dnsWait = aReq->dnsWait;
+ 
+ #if USE_ADAPTATION
+     adaptHistory_ = aReq->adaptHistory();
+ #endif
+ #if ICAP_CLIENT
+     icapHistory_ = aReq->icapHistory();
+ #endif
+ 
+     // This may be too conservative for the 204 No Content case
+     // may eventually need cloneNullAdaptationImmune() for that.
+     flags = aReq->flags.cloneAdaptationImmune();
+ 
+     errType = aReq->errType;
+     errDetail = aReq->errDetail;
+ #if USE_AUTH
+     auth_user_request = aReq->auth_user_request;
+ #endif
++
++    // main property is which connection the request was received on (if any)
+     clientConnectionManager = aReq->clientConnectionManager;
+     return true;
+ }
+ 
  /**
   * Checks the first line of an HTTP request is valid
   * currently just checks the request method is present.
diff --cc src/adaptation/icap/ServiceRep.cc
index 9d2f90f743,b5d17addb4..033b96b702
--- a/src/adaptation/icap/ServiceRep.cc
+++ b/src/adaptation/icap/ServiceRep.cc
@@@ -10,9 -10,11 +10,12 @@@
  #include "adaptation/icap/OptXact.h"
  #include "adaptation/icap/ServiceRep.h"
  #include "base/TextException.h"
++#include "comm/Connection.h"
  #include "ConfigParser.h"
+ #include "ip/tools.h"
  #include "HttpReply.h"
  #include "SquidTime.h"
+ #include "fde.h"
  
  CBDATA_NAMESPACED_CLASS_INIT(Adaptation::Icap, ServiceRep);
  
@@@ -72,6 -80,157 +81,155 @@@ void Adaptation::Icap::ServiceRep::note
      // should be configurable.
  }
  
+ // returns a persistent or brand new connection; negative int on failures
 -int Adaptation::Icap::ServiceRep::getConnection(bool retriableXact, bool &reused)
++Comm::ConnectionPointer
++Adaptation::Icap::ServiceRep::getConnection(bool retriableXact, bool &reused)
+ {
 -    Ip::Address client_addr;
++    Comm::ConnectionPointer connection = new Comm::Connection;
+ 
 -    int connection = theIdleConns.pop(cfg().host.termedBuf(), cfg().port, NULL, client_addr,
 -                                      retriableXact);
++    /* NP: set these here because it applies whether a pconn or a new conn is used */
+ 
 -    reused = connection >= 0; // reused a persistent connection
++    // TODO: Avoid blocking lookup if s.cfg().host is a hostname
++    connection->remote = cfg().host.termedBuf();
++    connection->remote.SetPort(cfg().port);
+ 
 -    if (!reused) { // need a new connection
 -        Ip::Address outgoing;  // default: IP6_ANY_ADDR
 -        if (!Ip::EnableIpv6)
 -            outgoing.SetIPv4();
 -        else if (Ip::EnableIpv6&IPV6_SPECIAL_SPLITSTACK &&  !cfg().ipv6) {
 -            /* split-stack for now requires default IPv4-only socket */
 -            outgoing.SetIPv4();
 -        }
 -        connection = comm_open(SOCK_STREAM, 0, outgoing, COMM_NONBLOCKING, cfg().uri.termedBuf());
 -    }
++    // TODO: check whether NULL domain is appropriate here
++    theIdleConns.pop(connection, NULL, retriableXact);
++    reused = connection->isOpen(); // reused a persistent connection
++
++    if (reused)
++        debugs(93,3, HERE << "reused pconn " << connection);
++    // else, return unopened Comm::Connection for caller to open.
+ 
 -    if (connection >= 0)
++    if (connection->isOpen())
+         ++theBusyConns;
+ 
+     return connection;
+ }
+ 
+ // pools connection if it is reusable or closes it
 -void Adaptation::Icap::ServiceRep::putConnection(int fd, bool isReusable, const char *comment)
++void Adaptation::Icap::ServiceRep::putConnection(const Comm::ConnectionPointer &conn, bool isReusable, const char *comment)
+ {
 -    Must(fd >= 0);
++    Must(Comm::IsConnOpen(conn));
+     // do not pool an idle connection if we owe connections
+     if (isReusable && excessConnections() == 0) {
+         debugs(93, 3, HERE << "pushing pconn" << comment);
 -        commSetTimeout(fd, -1, NULL, NULL);
 -        Ip::Address anyAddr;
 -        theIdleConns.push(fd, cfg().host.termedBuf(), cfg().port, NULL, anyAddr);
++        commUnsetConnTimeout(conn);
++        theIdleConns.push(conn, NULL);
+     } else {
+         debugs(93, 3, HERE << "closing pconn" << comment);
+         // comm_close will clear timeout
 -        comm_close(fd);
++        conn->close();
+     }
+ 
+     Must(theBusyConns > 0);
+     --theBusyConns;
+     // a connection slot released. Check if there are waiters....
+     busyCheckpoint();
+ }
+ 
+ // a wrapper to avoid exposing theIdleConns
 -void Adaptation::Icap::ServiceRep::noteConnectionUse(int fd)
++void Adaptation::Icap::ServiceRep::noteConnectionUse(const Comm::ConnectionPointer &conn)
+ {
 -    Must(fd >= 0);
 -    fd_table[fd].noteUse(&theIdleConns);
++    Must(Comm::IsConnOpen(conn));
++    fd_table[conn->fd].noteUse(&theIdleConns);
+ }
+ 
+ void Adaptation::Icap::ServiceRep::setMaxConnections()
+ {
+     if (cfg().maxConn >= 0)
+         theMaxConnections = cfg().maxConn;
+     else if (theOptions && theOptions->max_connections >= 0)
+         theMaxConnections = theOptions->max_connections;
+     else {
+         theMaxConnections = -1;
+         return;
+     }
+ 
+     if (::Config.workers > 1 )
+         theMaxConnections /= ::Config.workers;
+ }
+ 
+ int Adaptation::Icap::ServiceRep::availableConnections() const
+ {
+     if (theMaxConnections < 0)
+         return -1;
+ 
+     // we are available if we can open or reuse connections
+     // in other words, if we will not create debt
+     int available = max(0, theMaxConnections - theBusyConns);
+ 
+     if (!available && !connOverloadReported) {
+         debugs(93, DBG_IMPORTANT, "WARNING: ICAP Max-Connections limit " <<
+                "exceeded for service " << cfg().uri << ". Open connections now: " <<
+                theBusyConns + theIdleConns.count() << ", including " <<
+                theIdleConns.count() << " idle persistent connections.");
+         connOverloadReported = true;
+     }
+ 
+     if (cfg().onOverload == srvForce)
+         return -1;
+ 
+     return available;
+ }
+ 
+ // The number of connections which excess the Max-Connections limit
+ int Adaptation::Icap::ServiceRep::excessConnections() const
+ {
+     if (theMaxConnections < 0)
+         return 0;
+ 
+     // Waiters affect the number of needed connections but a needed
+     // connection may still be excessive from Max-Connections p.o.v.
+     // so we should not account for waiting transaction needs here.
+     const int debt =  theBusyConns + theIdleConns.count() - theMaxConnections;
+     if (debt > 0)
+         return debt;
+     else
+         return 0;
+ }
+ 
+ void Adaptation::Icap::ServiceRep::noteGoneWaiter()
+ {
+     theAllWaiters--;
+ 
+     // in case the notified transaction did not take the connection slot
+     busyCheckpoint();
+ }
+ 
+ // called when a connection slot may become available
+ void Adaptation::Icap::ServiceRep::busyCheckpoint()
+ {
+     if (theNotificationWaiters.empty()) // nobody is waiting for a slot
+         return;
+ 
+     int freed = 0;
+     int available = availableConnections();
+ 
+     if (available < 0) {
+         // It is possible to have waiters when no limit on connections exist in
+         // case of reconfigure or because new Options received.
+         // In this case, notify all waiting transactions.
+         freed  = theNotificationWaiters.size();
+     } else {
+         // avoid notifying more waiters than there will be available slots
+         const int notifiedWaiters = theAllWaiters - theNotificationWaiters.size();
+         freed = available - notifiedWaiters;
+     }
+ 
+     debugs(93,7, HERE << "Available connections: " << available <<
+            " freed slots: " << freed <<
+            " waiting in queue: " << theNotificationWaiters.size());
+ 
+     while (freed > 0 && !theNotificationWaiters.empty()) {
+         Client i = theNotificationWaiters.front();
+         theNotificationWaiters.pop_front();
+         ScheduleCallHere(i.callback);
+         i.callback = NULL;
+         --freed;
+     }
+ }
+ 
  void Adaptation::Icap::ServiceRep::suspend(const char *reason)
  {
      if (isSuspended) {
@@@ -351,6 -547,17 +546,18 @@@ void Adaptation::Icap::ServiceRep::hand
      debugs(93,3, HERE << "got new options and is now " << status());
  
      scheduleUpdate(optionsFetchTime());
+ 
++    // XXX: this whole feature bases on the false assumption a service only has one IP
+     setMaxConnections();
+     const int excess = excessConnections();
+     // if we owe connections and have idle pconns, close the latter
++    // XXX:  but ... idle pconn to *where*?
+     if (excess && theIdleConns.count() > 0) {
+         const int n = min(excess, theIdleConns.count());
+         debugs(93,5, HERE << "closing " << n << " pconns to relief debt");
 -        Ip::Address anyAddr;
 -        theIdleConns.closeN(n, cfg().host.termedBuf(), cfg().port, NULL, anyAddr);
++        theIdleConns.closeN(n, Comm::ConnectionPointer(), cfg().host.termedBuf());
+     }
+ 
      scheduleNotification();
  }
  
diff --cc src/adaptation/icap/ServiceRep.h
index bbf0f9178d,27caf2f24b..b17e0f7fee
--- a/src/adaptation/icap/ServiceRep.h
+++ b/src/adaptation/icap/ServiceRep.h
@@@ -104,6 -110,9 +110,9 @@@ public
      bool wantsPreview(const String &urlPath, size_t &wantedSize) const;
      bool allows204() const;
      bool allows206() const;
 -    int getConnection(bool isRetriable, bool &isReused);
 -    void putConnection(int fd, bool isReusable, const char *comment);
 -    void noteConnectionUse(int fd);
++    Comm::ConnectionPointer getConnection(bool isRetriable, bool &isReused);
++    void putConnection(const Comm::ConnectionPointer &conn, bool isReusable, const char *comment);
++    void noteConnectionUse(const Comm::ConnectionPointer &conn);
  
      void noteFailure(); // called by transactions to report service failure
  
diff --cc src/adaptation/icap/Xaction.cc
index 9cd3ae5ea3,4c39db104e..96561048a6
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@@ -88,28 -86,26 +86,25 @@@ void Adaptation::Icap::Xaction::start(
  }
  
  // TODO: obey service-specific, OPTIONS-reported connection limit
 -void Adaptation::Icap::Xaction::openConnection()
 +void
 +Adaptation::Icap::Xaction::openConnection()
  {
 -    Must(connection < 0);
 +    Must(!haveConnection());
  
-     const Adaptation::Service &s = service();
+     Adaptation::Icap::ServiceRep &s = service();
  
      if (!TheConfig.reuse_connections)
          disableRetries(); // this will also safely drain pconn pool
  
-     connection = new Comm::Connection;
- 
-     /* NP: set these here because it applies whether a pconn or a new conn is used */
+     bool wasReused = false;
+     connection = s.getConnection(isRetriable, wasReused);
 -    if (connection < 0)
 -        dieOnConnectionFailure(); // throws
  
-     // TODO: Avoid blocking lookup if s.cfg().host is a hostname
-     connection->remote = s.cfg().host.termedBuf();
-     connection->remote.SetPort(s.cfg().port);
- 
-     // TODO: check whether NULL domain is appropriate here
-     icapPconnPool->pop(connection, NULL, isRetriable);
-     if (connection->isOpen()) {
-         debugs(93,3, HERE << "reused pconn " << connection);
 -    if (wasReused) {
++    if (wasReused && Comm::IsConnOpen(connection)) {
+         // Set comm Close handler
+         typedef CommCbMemFunT<Adaptation::Icap::Xaction, CommCloseCbParams> CloseDialer;
+         closer =  asyncCall(93, 5, "Adaptation::Icap::Xaction::noteCommClosed",
+                             CloseDialer(this,&Adaptation::Icap::Xaction::noteCommClosed));
 -        comm_add_close_handler(connection, closer);
++        comm_add_close_handler(connection->fd, closer);
  
          // fake the connect callback
          // TODO: can we sync call Adaptation::Icap::Xaction::noteCommConnected here instead?
@@@ -126,11 -122,26 +121,27 @@@
  
      disableRetries(); // we only retry pconn failures
  
 -
++    // Attempt to open a new connection...
+     debugs(93,3, typeName << " opens connection to " << s.cfg().host.termedBuf() << ":" << s.cfg().port);
+ 
+     // TODO: service bypass status may differ from that of a transaction
+     typedef CommCbMemFunT<Adaptation::Icap::Xaction, CommTimeoutCbParams> TimeoutDialer;
+     AsyncCall::Pointer timeoutCall =  asyncCall(93, 5, "Adaptation::Icap::Xaction::noteCommTimedout",
+                                       TimeoutDialer(this,&Adaptation::Icap::Xaction::noteCommTimedout));
+ 
 -    commSetTimeout(connection, TheConfig.connect_timeout(
++    commSetTimeout(connection->fd, TheConfig.connect_timeout(
+                        service().cfg().bypass), timeoutCall);
+ 
+     typedef CommCbMemFunT<Adaptation::Icap::Xaction, CommCloseCbParams> CloseDialer;
+     closer =  asyncCall(93, 5, "Adaptation::Icap::Xaction::noteCommClosed",
+                         CloseDialer(this,&Adaptation::Icap::Xaction::noteCommClosed));
 -    comm_add_close_handler(connection, closer);
++    comm_add_close_handler(connection->fd, closer);
+ 
      typedef CommCbMemFunT<Adaptation::Icap::Xaction, CommConnectCbParams> ConnectDialer;
 -    connector = asyncCall(93,3, "Adaptation::Icap::Xaction::noteCommConnected",
 -                          ConnectDialer(this, &Adaptation::Icap::Xaction::noteCommConnected));
 -    commConnectStart(connection, s.cfg().host.termedBuf(), s.cfg().port, connector);
 +    connector = JobCallback(93,3, ConnectDialer, this, Adaptation::Icap::Xaction::noteCommConnected);
 +    Comm::ConnOpener *cs = new Comm::ConnOpener(connection, connector, TheConfig.connect_timeout(service().cfg().bypass));
 +    cs->setHost(s.cfg().host.termedBuf());
 +    AsyncJob::Start(cs);
  }
  
  /*
@@@ -197,12 -197,7 +202,13 @@@ void Adaptation::Icap::Xaction::noteCom
      if (io.flag != COMM_OK)
          dieOnConnectionFailure(); // throws
  
 +    typedef CommCbMemFunT<Adaptation::Icap::Xaction, CommCloseCbParams> CloseDialer;
 +    closer =  asyncCall(93, 5, "Adaptation::Icap::Xaction::noteCommClosed",
 +                        CloseDialer(this,&Adaptation::Icap::Xaction::noteCommClosed));
 +    comm_add_close_handler(io.conn->fd, closer);
 +
-     fd_table[io.conn->fd].noteUse(icapPconnPool);
++// ??    fd_table[io.conn->fd].noteUse(icapPconnPool);
+     service().noteConnectionUse(connection);
  
      handleCommConnected();
  }
diff --cc src/client_side.cc
index 681c4d9cbe,6136fa8a20..35f75a7325
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@@ -2153,11 -2159,11 +2153,11 @@@ parseHttpRequest(ConnStateData *csd, Ht
      }
  
      /* Set method_p */
-     *method_p = HttpRequestMethod(&hp->buf[hp->m_start], &hp->buf[hp->m_end]+1);
+     *method_p = HttpRequestMethod(&hp->buf[hp->req.m_start], &hp->buf[hp->req.m_end]+1);
  
      /* deny CONNECT via accelerated ports */
 -    if (*method_p == METHOD_CONNECT && conn && conn->port && conn->port->accel) {
 -        debugs(33, DBG_IMPORTANT, "WARNING: CONNECT method received on " << conn->port->protocol << " Accelerator port " << conn->port->s.GetPort() );
 +    if (*method_p == METHOD_CONNECT && csd && csd->port && csd->port->accel) {
 +        debugs(33, DBG_IMPORTANT, "WARNING: CONNECT method received on " << csd->port->protocol << " Accelerator port " << csd->port->s.GetPort() );
          /* XXX need a way to say "this many character length string" */
          debugs(33, DBG_IMPORTANT, "WARNING: for request: " << hp->buf);
          hp->request_parse_status = HTTP_METHOD_NOT_ALLOWED;
diff --cc src/client_side.h
index aa5961f764,bcb4ccf3f4..ba11aee90a
--- a/src/client_side.h
+++ b/src/client_side.h
@@@ -40,7 -40,10 +40,8 @@@
  #include "BodyPipe.h"
  #include "comm.h"
  #include "CommCalls.h"
 -#include "eui/Eui48.h"
 -#include "eui/Eui64.h"
  #include "HttpControlMsg.h"
+ #include "HttpParser.h"
  #include "RefCount.h"
  #include "StoreIOBuffer.h"
  
diff --cc src/client_side_reply.cc
index a10ff3bff4,8b24aa4ae7..d542c94f69
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@@ -2067,15 -2055,15 +2057,14 @@@ clientReplyContext::sendMoreData (Store
          /* we've got to copy some data */
          assert(result.length <= next()->readBuffer.length);
          memcpy(buf, result.data, result.length);
-         body_buf = buf;
      }
  
 -    if (reqofs==0 && !logTypeIsATcpHit(http->logType)) {
 -        assert(fd >= 0); // the beginning of this method implies fd may be -1
 +    if (reqofs==0 && !logTypeIsATcpHit(http->logType) && Comm::IsConnOpen(conn->clientConnection)) {
          if (Ip::Qos::TheConfig.isHitTosActive()) {
 -            Ip::Qos::doTosLocalMiss(fd, http->request->hier.code);
 +            Ip::Qos::doTosLocalMiss(conn->clientConnection, http->request->hier.code);
          }
          if (Ip::Qos::TheConfig.isHitNfmarkActive()) {
 -            Ip::Qos::doNfmarkLocalMiss(fd, http->request->hier.code);
 +            Ip::Qos::doNfmarkLocalMiss(conn->clientConnection, http->request->hier.code);
          }
      }
  
diff --cc src/client_side_request.cc
index 5991bda97b,d51702a1da..0aee4f10fc
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@@ -1053,40 -1070,10 +1075,10 @@@ ClientRequestContext::clientRedirectDon
          }
      }
  
-     if (new_request) {
-         safe_free(http->uri);
-         http->uri = xstrdup(urlCanonical(new_request));
-         new_request->http_ver = old_request->http_ver;
-         new_request->header.append(&old_request->header);
-         new_request->client_addr = old_request->client_addr;
- #if FOLLOW_X_FORWARDED_FOR
-         new_request->indirect_client_addr = old_request->indirect_client_addr;
- #endif /* FOLLOW_X_FORWARDED_FOR */
-         new_request->my_addr = old_request->my_addr;
-         new_request->flags = old_request->flags;
-         new_request->flags.redirected = 1;
- #if USE_AUTH
-         new_request->auth_user_request = old_request->auth_user_request;
- #endif
-         if (old_request->body_pipe != NULL) {
-             new_request->body_pipe = old_request->body_pipe;
-             old_request->body_pipe = NULL;
-             debugs(61,2, HERE << "URL-rewriter diverts body_pipe " << new_request->body_pipe <<
-                    " from request " << old_request << " to " << new_request);
-         }
- 
-         new_request->content_length = old_request->content_length;
-         new_request->extacl_user = old_request->extacl_user;
-         new_request->extacl_passwd = old_request->extacl_passwd;
-         new_request->flags.proxy_keepalive = old_request->flags.proxy_keepalive;
-         HTTPMSGUNLOCK(old_request);
-         http->request = HTTPMSGLOCK(new_request);
-     }
- 
      /* FIXME PIPELINE: This is innacurate during pipelining */
  
 -    if (http->getConn() != NULL)
 -        fd_note(http->getConn()->fd, http->uri);
 +    if (http->getConn() != NULL && Comm::IsConnOpen(http->getConn()->clientConnection))
 +        fd_note(http->getConn()->clientConnection->fd, http->uri);
  
      assert(http->uri);
  
@@@ -1639,9 -1629,13 +1631,13 @@@ ClientHttpRequest::handleAdaptationFail
      ConnStateData * c = getConn();
      repContext->setReplyToError(ERR_ICAP_FAILURE, HTTP_INTERNAL_SERVER_ERROR,
                                  request->method, NULL,
 -                                (c != NULL ? c->peer : noAddr), request, NULL,
 +                                (c != NULL ? c->clientConnection->remote : noAddr), request, NULL,
+ #if USE_AUTH
                                  (c != NULL && c->auth_user_request != NULL ?
                                   c->auth_user_request : request->auth_user_request));
+ #else
+                                 NULL);
+ #endif
  
      request->detailError(ERR_ICAP_FAILURE, errDetail);
  
diff --cc src/fs/ufs/store_io_ufs.cc
index 5b84d74c5f,5b84d74c5f..20c2ca5b6a
--- a/src/fs/ufs/store_io_ufs.cc
+++ b/src/fs/ufs/store_io_ufs.cc
@@@ -345,9 -345,9 +345,9 @@@ UFSStoreState::readCompleted(const cha
  void
  UFSStoreState::writeCompleted(int errflag, size_t len, RefCount<WriteRequest> writeRequest)
  {
--    debugs(79, 3, "UFSStoreState::writeCompleted: dirno " << swap_dirn << ", fileno " <<
++    debugs(79, 3, HERE << "dirno " << swap_dirn << ", fileno " <<
             std::setfill('0') << std::hex << std::uppercase << std::setw(8) << swap_filen <<
--           ", len " << len );
++           ", len " << len);
      /*
       * DPW 2006-05-24
       * See doWrites() for why we don't update UFSStoreState::writing
@@@ -357,8 -357,8 +357,7 @@@
      offset_ += len;
  
      if (theFile->error()) {
--        debugs(79,2,HERE << "UFSStoreState::writeCompleted" <<
--               " detected an error, will try to close");
++        debugs(79,2,HERE << " detected an error, will try to close");
          tryClosing();
      }
  
diff --cc src/pconn.cc
index ca23bceeec,be9fe30c3c..234eea0a2b
--- a/src/pconn.cc
+++ b/src/pconn.cc
@@@ -92,25 -81,26 +92,28 @@@ IdleConnList::findIndexOf(const Comm::C
      return -1;
  }
  
 -void
 -IdleConnList::removeFD(int fd)
 +/** Remove the entry at specified index.
 + * \retval false The index is not an in-use entry.
 + */
 +bool
 +IdleConnList::removeAt(int index)
  {
 -    int index = findFDIndex(fd);
 -    if (index < 0) {
 -        debugs(48, 2, "IdleConnList::removeFD: FD " << fd << " NOT FOUND!");
 -        return;
 -    }
 -    debugs(48, 3, "IdleConnList::removeFD: found FD " << fd << " at index " << index);
 +    if (index < 0 || index >= size_)
 +        return false;
  
 -    for (; index < nfds - 1; index++)
 -        fds[index] = fds[index + 1];
 +    // shuffle the remaining entries to fill the new gap.
 +    for (; index < size_ - 1; index++)
 +        theList_[index] = theList_[index + 1];
 +    theList_[size_-1] = NULL;
  
 -    if (parent)
 -        parent->noteConnectionRemoved();
++    if (parent_)
++        parent_->noteConnectionRemoved();
+ 
 -    if (--nfds == 0) {
 -        debugs(48, 3, "IdleConnList::removeFD: deleting " << hashKeyStr(&hash));
 +    if (--size_ == 0) {
 +        debugs(48, 3, HERE << "deleting " << hashKeyStr(&hash));
          delete this;
      }
 +    return true;
  }
  
  void
@@@ -122,31 -111,27 +125,34 @@@ IdleConnList::clearHandlers(const Comm:
  }
  
  void
 -IdleConnList::push(int fd)
 +IdleConnList::push(const Comm::ConnectionPointer &conn)
  {
 -    if (nfds == nfds_alloc) {
 -        debugs(48, 3, "IdleConnList::push: growing FD array");
 -        nfds_alloc <<= 1;
 -        int *old = fds;
 -        fds = (int *)xmalloc(nfds_alloc * sizeof(int));
 -        memcpy(fds, old, nfds * sizeof(int));
 -
 -        if (nfds == PCONN_FDS_SZ)
 -            pconn_fds_pool->freeOne(old);
 +    if (size_ == capacity_) {
 +        debugs(48, 3, HERE << "growing idle Connection array");
 +        capacity_ <<= 1;
 +        const Comm::ConnectionPointer *oldList = theList_;
 +        theList_ = new Comm::ConnectionPointer[capacity_];
 +        for (int index = 0; index < size_; index++)
 +            theList_[index] = oldList[index];
 +
 +/* TODO: re-attach to MemPools.
 +        if (size_ == PCONN_FDS_SZ)
 +            pconn_fds_pool->freeOne(oldList);
          else
 -            xfree(old);
 +*/
 +        delete[] oldList;
      }
  
 -    if (parent)
 -        parent->noteConnectionAdded();
++    if (parent_)
++        parent_->noteConnectionAdded();
+ 
 -    fds[nfds++] = fd;
 -    comm_read(fd, fakeReadBuf, sizeof(fakeReadBuf), IdleConnList::read, this);
 -    commSetTimeout(fd, Config.Timeout.pconn, IdleConnList::timeout, this);
 +    theList_[size_++] = conn;
 +    AsyncCall::Pointer readCall = commCbCall(5,4, "IdleConnList::Read",
 +                                             CommIoCbPtrFun(IdleConnList::Read, this));
 +    comm_read(conn, fakeReadBuf_, sizeof(fakeReadBuf_), readCall);
 +    AsyncCall::Pointer timeoutCall = commCbCall(5,4, "IdleConnList::Read",
 +                                                CommTimeoutCbPtrFun(IdleConnList::Timeout, this));
 +    commSetConnTimeout(conn, Config.Timeout.pconn, timeoutCall);
  }
  
  /*
@@@ -355,8 -329,18 +362,18 @@@ PconnPool::pop(const Comm::ConnectionPo
  }
  
  void
- PconnPool::unlinkList(IdleConnList *list) const
 -PconnPool::closeN(int n, const char *host, u_short port, const char *domain, Ip::Address &client_address)
++PconnPool::closeN(int n, const Comm::ConnectionPointer &destLink, const char *domain)
+ {
+     // TODO: optimize: we can probably do hash_lookup just once
+     for (int i = 0; i < n; ++i)
 -        pop(host, port, domain, client_address, false); // may fail!
++        pop(destLink, domain, false); // may fail!
+ }
+ 
+ void
+ PconnPool::unlinkList(IdleConnList *list)
  {
+     theCount -= list->count();
+     assert(theCount >= 0);
      hash_remove_link(table, &list->hash);
  }
  
diff --cc src/pconn.h
index 26860b874f,6eada4fdbd..fc39a773d0
--- a/src/pconn.h
+++ b/src/pconn.h
@@@ -35,24 -33,17 +35,26 @@@ public
      IdleConnList(const char *key, PconnPool *parent);
      ~IdleConnList();
  
 -    int findFDIndex(int fd); ///< search from the end of array
 -    void removeFD(int fd);
 -    void push(int fd);
 -    int findUseableFD();     ///< find first from the end not pending read fd.
 -    void clearHandlers(int fd);
 +    /// Pass control of the connection to the idle list.
 +    void push(const Comm::ConnectionPointer &conn);
 +
 +    /** Search the list for a connection which matches the 'key' details
 +     * and pop it off the list.
 +     * The list is created based on remote IP:port hash. This further filters
 +     * the choices based on specific local-end details requested.
 +     * If nothing usable is found the a nil pointer is returned.
 +     */
 +    Comm::ConnectionPointer findUseable(const Comm::ConnectionPointer &key);
  
 -    int count() const { return nfds; }
 +    void clearHandlers(const Comm::ConnectionPointer &conn);
 +
++    int count() const { return size_; }
+ 
  private:
 -    static IOCB read;
 -    static PF timeout;
 +    bool removeAt(int index);
 +    int findIndexOf(const Comm::ConnectionPointer &conn) const;
 +    static IOCB Read;
 +    static CTCB Timeout;
  
  public:
      hash_link hash;             /** must be first */
@@@ -106,21 -81,16 +108,26 @@@ public
      ~PconnPool();
  
      void moduleInit();
 -    void push(int fd, const char *host, u_short port, const char *domain, Ip::Address &client_address);
 -    int pop(const char *host, u_short port, const char *domain, Ip::Address &client_address, bool retriable);
 -    void noteUses(int uses);
 -    void dumpHist(StoreEntry *e);
 -    void dumpHash(StoreEntry *e);
 +    void push(const Comm::ConnectionPointer &serverConn, const char *domain);
 +
 +    /**
 +     * Updates destLink to point at an existing open connection if available and retriable.
 +     * Otherwise, return false.
 +     *
 +     * We close available persistent connection if the caller transaction is not
 +     * retriable to avoid having a growing number of open connections when many
 +     * transactions create persistent connections but are not retriable.
 +     */
 +    Comm::ConnectionPointer pop(const Comm::ConnectionPointer &destLink, const char *domain, bool retriable);
 +    void count(int uses);
 +    void dumpHist(StoreEntry *e) const;
 +    void dumpHash(StoreEntry *e) const;
-     void unlinkList(IdleConnList *list) const;
+     void unlinkList(IdleConnList *list);
 -    void closeN(int n, const char *host, u_short port, const char *domain, Ip::Address &client_address);
++    void noteUses(int uses);
++    void closeN(int n, const Comm::ConnectionPointer &destLink, const char *domain);
+     int count() const { return theCount; }
+     void noteConnectionAdded() { ++theCount; }
+     void noteConnectionRemoved() { assert(theCount > 0); --theCount; }
  
  private:
  
diff --cc src/peer_select.cc
index 71301da523,b426510077..3201906395
--- a/src/peer_select.cc
+++ b/src/peer_select.cc
@@@ -413,17 -344,15 +413,17 @@@ peerSelectFoo(ps_state * ps
          break;
      }
  
 -    peerSelectCallback(ps);
 +    // resolve the possible peers
 +    peerSelectDnsPaths(ps);
  }
  
 +int peerAllowedToUse(const peer * p, HttpRequest * request);
 +
- /*
+ /**
   * peerSelectPinned
   *
-  * Selects a pinned connection
+  * Selects a pinned connection.
   */
 -int peerAllowedToUse(const peer * p, HttpRequest * request);
  static void
  peerSelectPinned(ps_state * ps)
  {
diff --cc src/snmp_core.h
index 2ce506f573,9dd716a50b..e66047133b
--- a/src/snmp_core.h
+++ b/src/snmp_core.h
@@@ -8,9 -8,7 +8,8 @@@
  #ifndef SQUID_SNMP_CORE_H
  #define SQUID_SNMP_CORE_H
  
- #include "config.h"
  #include "cache_snmp.h"
 +#include "comm/forward.h"
  
  #define SNMP_REQUEST_SIZE 4096
  #define MAX_PROTOSTAT 5
diff --cc src/tests/stub_pconn.cc
index 6d3fe3917d,0000000000..0affac1bd4
mode 100644,000000..100644
--- a/src/tests/stub_pconn.cc
+++ b/src/tests/stub_pconn.cc
@@@ -1,125 -1,0 +1,131 @@@
 +/*
 + * STUB file for the pconn.cc API
 + * Functions here are inactive.
 + */
 +#include "config.h"
 +#include "pconn.h"
 +#include "comm/Connection.h"
 +
 +IdleConnList::IdleConnList(const char *key, PconnPool *parent)
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +IdleConnList::~IdleConnList()
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +void
 +IdleConnList::push(const Comm::ConnectionPointer &conn)
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +Comm::ConnectionPointer
 +IdleConnList::findUseable(const Comm::ConnectionPointer &key)
 +{
 +    fatal("pconn.cc required");
 +    return Comm::ConnectionPointer();
 +}
 +
 +void
 +IdleConnList::clearHandlers(const Comm::ConnectionPointer &conn)
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +PconnPool::PconnPool(const char *)
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +PconnPool::~PconnPool()
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +void
 +PconnPool::moduleInit()
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +void
 +PconnPool::push(const Comm::ConnectionPointer &serverConn, const char *domain)
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +Comm::ConnectionPointer
 +PconnPool::pop(const Comm::ConnectionPointer &destLink, const char *domain, bool retriable)
 +{
 +    fatal("pconn.cc required");
 +    return Comm::ConnectionPointer();
 +}
 +
 +void
 +PconnPool::count(int uses)
 +{
 +    fatal("pconn.cc required");
 +}
 +
++void
++PconnPool::noteUses(int)
++{
++    fatal("pconn.cc required");
++}
++
 +void
 +PconnPool::dumpHist(StoreEntry *e) const
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +void
 +PconnPool::dumpHash(StoreEntry *e) const
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +void
- PconnPool::unlinkList(IdleConnList *list) const
++PconnPool::unlinkList(IdleConnList *list)
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +PconnModule *
 +PconnModule::GetInstance()
 +{
 +    fatal("pconn.cc required");
 +    return NULL;
 +}
 +
 +void
 +PconnModule::DumpWrapper(StoreEntry *e)
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +PconnModule::PconnModule()
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +void
 +PconnModule::registerWithCacheManager(void)
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +void
 +PconnModule::add(PconnPool *)
 +{
 +    fatal("pconn.cc required");
 +}
 +
 +void
 +PconnModule::dump(StoreEntry *)
 +{
 +    fatal("pconn.cc required");
 +}