Do not reuse an idle connection to a removed cache_peer (#2028)

Sending new requests to a removed cache_peer contradicts current Squid
configuration and even exposes Squid code that forgets to check
CachePeer validity to dangling pointers. We will address the latter
concern separately.

diff --git a/src/comm/Connection.cc b/src/comm/Connection.cc
index 4ab4b056ead646cc060a18edbd9d0b85f941f20d..93a460c048486c636aaae2631dac7b458360210b 100644 (file)
--- a/src/comm/Connection.cc
+++ b/src/comm/Connection.cc
@@ -139,6 +139,12 @@ Comm::Connection::setPeer(CachePeer *p)
     }
 }
 
+bool
+Comm::Connection::toGoneCachePeer() const
+{
+    return peer_ && !cbdataReferenceValid(peer_);
+}
+
 time_t
 Comm::Connection::timeLeft(const time_t idleTimeout) const
 {

diff --git a/src/comm/Connection.h b/src/comm/Connection.h
index cb0a4a1f7dd4abec5a0ce4a608bb85715a0b4e9c..83f053d790e6a224d8df2a62c6f2010bc456fab9 100644 (file)
--- a/src/comm/Connection.h
+++ b/src/comm/Connection.h
@@ -116,6 +116,9 @@ public:
      */
     void setPeer(CachePeer * p);
 
+    /// whether this is a connection to a cache_peer that was removed during reconfiguration
+    bool toGoneCachePeer() const;
+
     /** The time the connection started */
     time_t startTime() const {return startTime_;}
 

diff --git a/src/pconn.cc b/src/pconn.cc
index db943ccec346b16ea212b8fc2fdcf36b64f4e0c4..a5b0d5bc903154fa0d5e929eef6a8faeca988b60 100644 (file)
--- a/src/pconn.cc
+++ b/src/pconn.cc
@@ -227,6 +227,11 @@ IdleConnList::pop()
         if (fd_table[theList_[i]->fd].timeoutHandler == nullptr)
             continue;
 
+        // the cache_peer has been removed from the configuration
+        // TODO: remove all such connections at once during reconfiguration
+        if (theList_[i]->toGoneCachePeer())
+            continue;
+
         // finally, a match. pop and return it.
         Comm::ConnectionPointer result = theList_[i];
         clearHandlers(result);
@@ -274,6 +279,11 @@ IdleConnList::findUseable(const Comm::ConnectionPointer &aKey)
         if (fd_table[theList_[i]->fd].timeoutHandler == nullptr)
             continue;
 
+        // the cache_peer has been removed from the configuration
+        // TODO: remove all such connections at once during reconfiguration
+        if (theList_[i]->toGoneCachePeer())
+            continue;
+
         // finally, a match. pop and return it.
         Comm::ConnectionPointer result = theList_[i];
         clearHandlers(result);