From: Christos Tsantilas <chtsanti@users.sourceforge.net>
Date: Mon, 23 Jan 2017 02:05:46 +0000 (+1300)
Subject: Reduce crashes due to unexpected ClientHttpRequest termination.
X-Git-Tag: SQUID_3_5_24~9
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=86c4647f72ee110068e71692675d85a26be90b09;p=thirdparty%2Fsquid.git

Reduce crashes due to unexpected ClientHttpRequest termination.

The underlying problem has been known since r13480: If a
ClientHttpRequest job ends without Http::Stream (and ConnStateData)
knowledge, then Squid is likely to segfault or assert. This patch does
not resolve the underlying issue (a proper fix would require
architectural changes in a consensus-lacking area) but makes an
unexpected ClientHttpRequest job destruction less likely.

BodyPipe and Adaptation-related exceptions are the major causes of
unexpected ClientHttpRequest job destruction. This patch handles them by
closing the client connection. Connection closure should trigger an
orderly top-down cleanup, including Http::Stream, ConnStateData, and
ClientHttpRequest destruction.

If there is no connection to close, then the exception is essentially
ignored with a level-1 error message disclosing the problem. The side
effects of ignoring such exceptions are unknown, but without a client
connection, it is our hope that they would be relatively benign.

This is a Measurement Factory project.
---

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index aaac805de9..cf22744083 100644
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -2080,5 +2080,21 @@ ClientHttpRequest::handleAdaptationFailure(int errDetail, bool bypassable)
     //else if(calloutContext == NULL) is it possible?
 }
 
+void
+ClientHttpRequest::callException(const std::exception &ex)
+{
+    const Comm::ConnectionPointer clientConn = getConn() ? getConn()->clientConnection : NULL;
+    if (Comm::IsConnOpen(clientConn)) {
+        debugs(85, 3, "closing after exception: " << ex.what());
+        clientConn->close(); // initiate orderly top-to-bottom cleanup
+        return;
+    }
+
+    debugs(85, DBG_IMPORTANT, "ClientHttpRequest exception without connection. Ignoring " << ex.what());
+    // XXX: Normally, we mustStop() but we cannot do that here because it is
+    // likely to leave Http::Stream and ConnStateData with a dangling http
+    // pointer. See r13480 or XXX in Http::Stream class description.
+}
+
 #endif
 
diff --git a/src/client_side_request.h b/src/client_side_request.h
index 50289a2fe7..31e87ed5b5 100644
--- a/src/client_side_request.h
+++ b/src/client_side_request.h
@@ -113,6 +113,7 @@ public:
         return Initiator::doneAll() &&
                BodyConsumer::doneAll() && false;
     }
+    virtual void callException(const std::exception &ex);
 #endif
 
 private: