Do not use invasive lists to store CachePeers (#1424)

[thirdparty/squid.git] / src / peer_select.cc

diff --git a/src/peer_select.cc b/src/peer_select.cc
index 9387fa3643cb289e1d74eeb6fe87f4519d785fbd..c1e0d89612c300a1254f00618bc783789b0a1caa 100644 (file)
--- a/src/peer_select.cc
+++ b/src/peer_select.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 1996-2022 The Squid Software Foundation and contributors
+ * Copyright (C) 1996-2023 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
@@ -14,6 +14,7 @@
 #include "base/InstanceId.h"
 #include "base/TypeTraits.h"
 #include "CachePeer.h"
+#include "CachePeers.h"
 #include "carp.h"
 #include "client_side.h"
 #include "dns/LookupDetails.h"
@@ -94,7 +95,7 @@ operator <<(std::ostream &os, const PeerSelectionDumper &fsd)
     os << hier_code_str[fsd.code];
 
     if (fsd.peer)
-        os << '/' << fsd.peer->host;
+        os << '/' << *fsd.peer;
     else if (fsd.selector) // useful for DIRECT and gone PINNED destinations
         os << '#' << fsd.selector->request->url.host();
 
@@ -252,7 +253,7 @@ PeerSelector::~PeerSelector()
     if (entry) {
         assert(entry->ping_status != PING_WAITING);
         entry->unlock("peerSelect");
-        entry = NULL;
+        entry = nullptr;
     }
 
     delete lastError;
@@ -530,7 +531,10 @@ PeerSelector::noteIp(const Ip::Address &ip)
 
     Comm::ConnectionPointer p = new Comm::Connection();
     p->remote = ip;
-    p->remote.port(peer ? peer->http_port : request->url.port());
+    // XXX: We return a (non-peer) destination with a zero port if the selection
+    // initiator supplied a request target without a port. If there are no valid
+    // use cases for this behavior, stop _selecting_ such destinations.
+    p->remote.port(peer ? peer->http_port : request->url.port().value_or(0));
     handlePath(p, *servers);
 }
 
@@ -545,7 +549,7 @@ PeerSelector::noteIps(const Dns::CachedIps *ia, const Dns::LookupDetails &detail
         debugs(44, 3, "Unknown host: " << (fs->_peer.valid() ? fs->_peer->host : request->url.host()));
         // discard any previous error.
         delete lastError;
-        lastError = NULL;
+        lastError = nullptr;
         if (fs->code == HIER_DIRECT) {
             lastError = new ErrorState(ERR_DNS_FAIL, Http::scServiceUnavailable, request, al);
             lastError->dnsError = details.error;
@@ -590,7 +594,7 @@ PeerSelector::checkNetdbDirect()
 
     p = whichPeer(closest_parent_miss);
 
-    if (p == NULL)
+    if (p == nullptr)
         return 0;
 
     debugs(44, 3, "closest_parent_miss RTT = " << ping.p_rtt << " msec");
@@ -616,7 +620,7 @@ PeerSelector::selectMore()
         if (always_direct == ACCESS_DUNNO) {
             debugs(44, 3, "direct = " << DirectStr[direct] << " (always_direct to be checked)");
             /** check always_direct; */
-            ACLFilledChecklist *ch = new ACLFilledChecklist(Config.accessList.AlwaysDirect, request, NULL);
+            ACLFilledChecklist *ch = new ACLFilledChecklist(Config.accessList.AlwaysDirect, request, nullptr);
             ch->al = al;
             acl_checklist = ch;
             acl_checklist->syncAle(request, nullptr);
@@ -625,7 +629,7 @@ PeerSelector::selectMore()
         } else if (never_direct == ACCESS_DUNNO) {
             debugs(44, 3, "direct = " << DirectStr[direct] << " (never_direct to be checked)");
             /** check never_direct; */
-            ACLFilledChecklist *ch = new ACLFilledChecklist(Config.accessList.NeverDirect, request, NULL);
+            ACLFilledChecklist *ch = new ACLFilledChecklist(Config.accessList.NeverDirect, request, nullptr);
             ch->al = al;
             acl_checklist = ch;
             acl_checklist->syncAle(request, nullptr);
@@ -652,7 +656,7 @@ PeerSelector::selectMore()
 
     if (!entry || entry->ping_status == PING_NONE)
         selectPinned();
-    if (entry == NULL) {
+    if (entry == nullptr) {
         (void) 0;
     } else if (entry->ping_status == PING_NONE) {
         selectSomeNeighbor();
@@ -784,7 +788,7 @@ PeerSelector::selectSomeNeighbor()
 void
 PeerSelector::selectSomeNeighborReplies()
 {
-    CachePeer *p = NULL;
+    CachePeer *p = nullptr;
     hier_code code = HIER_NONE;
     assert(entry->ping_status == PING_WAITING);
     assert(direct != DIRECT_YES);
@@ -862,10 +866,10 @@ PeerSelector::selectSomeParent()
 void
 PeerSelector::selectAllParents()
 {
-    CachePeer *p;
     /* Add all alive parents */
 
-    for (p = Config.peers; p; p = p->next) {
+    for (const auto &peer: CurrentCachePeers()) {
+        const auto p = peer.get();
         /* XXX: neighbors.c lacks a public interface for enumerating
          * parents to a request so we have to dig some here..
          */
@@ -884,7 +888,7 @@ PeerSelector::selectAllParents()
      * simply are not configured to handle the request.
      */
     /* Add default parent as a last resort */
-    if ((p = getDefaultParent(this))) {
+    if (const auto p = getDefaultParent(this)) {
         addSelection(p, DEFAULT_PARENT);
     }
 }
@@ -1105,17 +1109,17 @@ PeerSelector::addSelection(CachePeer *peer, const hier_code code)
 
 PeerSelector::PeerSelector(PeerSelectionInitiator *initiator):
     request(nullptr),
-    entry (NULL),
+    entry (nullptr),
     always_direct(Config.accessList.AlwaysDirect?ACCESS_DUNNO:ACCESS_DENIED),
     never_direct(Config.accessList.NeverDirect?ACCESS_DUNNO:ACCESS_DENIED),
     direct(DIRECT_UNKNOWN),
-    lastError(NULL),
-    servers (NULL),
+    lastError(nullptr),
+    servers (nullptr),
     first_parent_miss(),
     closest_parent_miss(),
-    hit(NULL),
+    hit(nullptr),
     hit_type(PEER_NONE),
-    acl_checklist (NULL),
+    acl_checklist (nullptr),
     initiator_(initiator)
 {
     ; // no local defaults.