From: Christos Tsantilas <chtsanti@users.sourceforge.net>
Date: Wed, 11 Jan 2017 18:17:47 +0000 (+0200)
Subject: Reduce crashes due to unexpected ClientHttpRequest termination.
X-Git-Tag: M-staged-PR71~312
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9d52ba1165dfa797b4fe688b49f0c10dc98e25a5;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 b43acdd0eb..81e193fd48 100644
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -2065,6 +2065,21 @@ ClientHttpRequest::handleAdaptationFailure(int errDetail, bool bypassable)
         doCallouts();
 }
 
+void
+ClientHttpRequest::callException(const std::exception &ex)
+{
+    if (const auto clientConn = getConn() ? getConn()->clientConnection : nullptr) {
+        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
 
 // XXX: modify and use with ClientRequestContext::clientAccessCheckDone too.
diff --git a/src/client_side_request.h b/src/client_side_request.h
index 2ba414cd27..df6462a6d2 100644
--- a/src/client_side_request.h
+++ b/src/client_side_request.h
@@ -131,6 +131,7 @@ public:
         return Initiator::doneAll() &&
                BodyConsumer::doneAll() && false;
     }
+    virtual void callException(const std::exception &ex);
 #endif
 
 private: