From 67c1efe95a00b964acbccdce2dca4fef41497a8a Mon Sep 17 00:00:00 2001
From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Thu, 15 Jul 2021 14:34:43 -0400
Subject: [PATCH] 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.
---
 src/security/PeerConnector.cc | 24 ++++++------------------
 src/security/PeerConnector.h  |  2 --
 2 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc
index 3a6794d44e..d0cfeb8136 100644
--- 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 cf58d85436..cb6cb0f642 100644
--- 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.
-- 
2.47.3