From f21e7f1affd75f4fce82a929223f4c0ca4ca5c9b Mon Sep 17 00:00:00 2001
From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Wed, 14 Jul 2021 16:58:10 -0400
Subject: [PATCH] Fix Tunneler handling of last-resort callback requirements

... and simplify the corresponding code.

Events like noteAbort() and callException() terminate the job but are
not bugs that should be reported by swanSong().

Not treating (rare) cancelled callbacks specially in swanSong() allows
us to avoid making cleanup code even more complex than it already is,
undoing branch changes that introduced closeQuietly().
---
 src/clients/HttpTunneler.cc | 24 ++++++------------------
 src/clients/HttpTunneler.h  |  4 ----
 2 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/src/clients/HttpTunneler.cc b/src/clients/HttpTunneler.cc
index 3ae727bda1..f4ceb2d2e8 100644
--- a/src/clients/HttpTunneler.cc
+++ b/src/clients/HttpTunneler.cc
@@ -364,14 +364,8 @@ Http::Tunneler::bailWith(ErrorState *error)
         peerConnectFailed(p);
 
     callBack();
-
-    closeQuietly();
-}
-
-void
-Http::Tunneler::closeQuietly()
-{
     disconnect();
+
     if (noteFwdPconnUse)
         fwdPconnPool->noteUses(fd_table[connection->fd].pconn.uses);
     // TODO: Reuse to-peer connections after a CONNECT error response.
@@ -421,19 +415,13 @@ Http::Tunneler::swanSong()
     AsyncJob::swanSong();
 
     if (callback) {
-        if (callback->canceled()) {
-            debugs(83, 3, "cancelled by the caller");
-            closeQuietly();
+        if (requestWritten && tunnelEstablished) {
+            sendSuccess();
         } else {
-            if (requestWritten && tunnelEstablished) {
-                sendSuccess();
-            } else {
-                // we should have bailed when we discovered the job-killing problem
-                debugs(83, DBG_IMPORTANT, "BUG: Unexpected state while establishing a CONNECT tunnel " << connection << status());
-                bailWith(new ErrorState(ERR_GATEWAY_FAILURE, Http::scInternalServerError, request.getRaw(), al));
-            }
-            assert(!callback);
+            // job-ending emergencies like noteAbort() or callException()
+            bailWith(new ErrorState(ERR_GATEWAY_FAILURE, Http::scInternalServerError, request.getRaw(), al));
         }
+        assert(!callback);
     }
 }
 
diff --git a/src/clients/HttpTunneler.h b/src/clients/HttpTunneler.h
index 00f9228c5f..08803eed99 100644
--- a/src/clients/HttpTunneler.h
+++ b/src/clients/HttpTunneler.h
@@ -34,8 +34,6 @@ class Tunneler: virtual public AsyncJob
     CBDATA_CLASS(Tunneler);
 
 public:
-    typedef CbcPointer<Tunneler> Pointer;
-
     /// Callback dialer API to allow Tunneler to set the answer.
     template <class Initiator>
     class CbDialer: public CallDialer, public Http::TunnelerAnswer
@@ -104,8 +102,6 @@ protected:
     TunnelerAnswer &answer();
 
 private:
-    void closeQuietly();
-
     AsyncCall::Pointer writer; ///< called when the request has been written
     AsyncCall::Pointer reader; ///< called when the response should be read
     AsyncCall::Pointer closer; ///< called when the connection is being closed
-- 
2.47.2