Fix PeerConnector handling of last-resort callback requirements

... and simplify the corresponding code.

Events like handleStopRequest() and callException() stop the job but
should not be reported as a BUG (e.g., it would be up to the
callException() to decide how to report the caught exception). There
might be (or there will) be other, similar cases where the job is
stopped prematurely for some non-BUG reason beyond swanSong() knowledge.

The existence of non-bug cases does not mean there could be no bugs
worth reporting here, but until they can be identified more reliably
than all these benign/irrelevant cases, reporting no BUGs is a (much)
lesser evil.

diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc
index 3a6794d44e0b2edee14bcbee264ed52fd1581335..d0cfeb81368b33f5230e1d7631a53efc5ab28667 100644 (file)
--- a/src/security/PeerConnector.cc
+++ b/src/security/PeerConnector.cc
@@ -551,14 +551,8 @@ Security::PeerConnector::bail(ErrorState *error)
         peerConnectFailed(p);
 
     callBack();
-
-    closeQuietly();
-}
-
-void
-Security::PeerConnector::closeQuietly()
-{
     disconnect();
+
     if (noteFwdPconnUse)
         fwdPconnPool->noteUses(fd_table[serverConn->fd].pconn.uses);
     serverConn->close();
@@ -604,19 +598,13 @@ Security::PeerConnector::swanSong()
     // XXX: unregister fd-closure monitoring and CommSetSelect interest, if any
     AsyncJob::swanSong();
 
-    if (!callback)
-        return;
-
-    if (callback->canceled()) {
-        debugs(83, 3, "cancelled by the caller");
-        closeQuietly();
+    if (callback) {
+        // job-ending emergencies like handleStopRequest() or callException()
+        const auto anErr = new ErrorState(ERR_GATEWAY_FAILURE, Http::scInternalServerError, request.getRaw(), al);
+        bail(anErr);
+        assert(!callback);
         return;
     }
-    // paranoid: we have left the caller waiting
-    debugs(83, DBG_IMPORTANT, "BUG: Unexpected state while connecting to a cache_peer or origin server");
-    const auto anErr = new ErrorState(ERR_GATEWAY_FAILURE, Http::scInternalServerError, request.getRaw(), al);
-    bail(anErr);
-    assert(!callback);
 }
 
 const char *

diff --git a/src/security/PeerConnector.h b/src/security/PeerConnector.h
index cf58d8543686fbb6f7eda761fc985fa13874547d..cb6cb0f64227a51da653cf4b1c11b988bce6764b 100644 (file)
--- a/src/security/PeerConnector.h
+++ b/src/security/PeerConnector.h
@@ -176,8 +176,6 @@ private:
     static void NegotiateSsl(int fd, void *data);
     void negotiateSsl();
 
-    void closeQuietly();
-
     /// The maximum allowed missing certificates downloads.
     static const unsigned int MaxCertsDownloads = 10;
     /// The maximum allowed nested certificates downloads.