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 b43acdd0eb0f3b19399c6a8af377dce781bd5283..81e193fd480f46bec43ce23cef1b757c2abf4637 100644 (file)
--- 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 2ba414cd2780c603bff890d3476f4bfb23c36f24..df6462a6d295f7184ded516f2e480cbf732d29b9 100644 (file)
--- 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: