AsyncJob-protect and reduce ICAP DNS lookup call chain (#1112)

Co-authored-by: Alex Rousskov <rousskov@measurement-factory.com>

diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index 08572b782cf50c282b9aab02ef83304c6fb22c3a..3dea5e54efc11901ec6404d51e206630df09b5f8 100644 (file)
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -14,6 +14,7 @@
 #include "adaptation/icap/Launcher.h"
 #include "adaptation/icap/Xaction.h"
 #include "base/JobWait.h"
+#include "base/Optional.h"
 #include "base/TextException.h"
 #include "comm.h"
 #include "comm/Connection.h"
@@ -146,9 +147,8 @@ static void
 icapLookupDnsResults(const ipcache_addrs *ia, const Dns::LookupDetails &, void *data)
 {
     Adaptation::Icap::Xaction *xa = static_cast<Adaptation::Icap::Xaction *>(data);
-    /// TODO: refactor with CallJobHere1, passing either std::optional (after upgrading to C++17)
-    /// or Optional<Ip::Address> (when it can take non-trivial types)
-    xa->dnsLookupDone(ia);
+    const auto &addr = ia ? Optional<Ip::Address>(ia->current()) : Optional<Ip::Address>();
+    CallJobHere1(93, 5, CbcPointer<Adaptation::Icap::Xaction>(xa), Adaptation::Icap::Xaction, dnsLookupDone, addr);
 }
 
 // TODO: obey service-specific, OPTIONS-reported connection limit
@@ -179,14 +179,14 @@ Adaptation::Icap::Xaction::openConnection()
 }
 
 void
-Adaptation::Icap::Xaction::dnsLookupDone(const ipcache_addrs *ia)
+Adaptation::Icap::Xaction::dnsLookupDone(Optional<Ip::Address> addr)
 {
     assert(waitingForDns);
     waitingForDns = false;
 
     Adaptation::Icap::ServiceRep &s = service();
 
-    if (ia == nullptr) {
+    if (!addr.has_value()) {
         debugs(44, DBG_IMPORTANT, "ERROR: ICAP: Unknown service host: " << s.cfg().host);
 
 #if WHEN_IPCACHE_NBGETHOSTBYNAME_USES_ASYNC_CALLS
@@ -198,7 +198,7 @@ Adaptation::Icap::Xaction::dnsLookupDone(const ipcache_addrs *ia)
     }
 
     const Comm::ConnectionPointer conn = new Comm::Connection();
-    conn->remote = ia->current();
+    conn->remote = addr.value();
     conn->remote.port(s.cfg().port);
     getOutgoingAddress(nullptr, conn);
 

diff --git a/src/adaptation/icap/Xaction.h b/src/adaptation/icap/Xaction.h
index 31a6e22fc9ea274a2efc01e4977fa68b36b22643..defdb8dd51a7cfd11af0952879f72e7696533d79 100644 (file)
--- a/src/adaptation/icap/Xaction.h
+++ b/src/adaptation/icap/Xaction.h
@@ -117,7 +117,7 @@ public:
     /// clear stored error details, if any; used for retries/repeats
     virtual void clearError() {}
     virtual AccessLogEntry::Pointer masterLogEntry();
-    void dnsLookupDone(const ipcache_addrs *ia);
+    void dnsLookupDone(Optional<Ip::Address>);
 
 protected:
     // logging

diff --git a/src/base/Optional.h b/src/base/Optional.h
index 72440e8e105f35b41bcbf80aaa3fc7f581d8eb6e..a803d30d8cf85b5cbeacdfcdeaac8f533890f0b5 100644 (file)
--- a/src/base/Optional.h
+++ b/src/base/Optional.h
@@ -10,6 +10,7 @@
 #define SQUID__SRC_BASE_OPTIONAL_H
 
 #include <exception>
+#include <ostream>
 #include <type_traits>
 #include <utility>
 
@@ -121,5 +122,16 @@ private:
     bool hasValue_ = false;
 };
 
+template <typename Value>
+inline
+std::ostream &operator <<(std::ostream &os, const Optional<Value> &opt)
+{
+    if (opt.has_value())
+        os << opt.value();
+    else
+        os << "[no value]";
+    return os;
+}
+
 #endif /* SQUID__SRC_BASE_OPTIONAL_H */