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 aaac805de9a41a879b521a2fd9265b09f287b3f7..cf227440838aa0a09af0e3658544156453cbed29 100644 (file)
--- 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 50289a2fe7a46a332953f094d19e1e50dfc30c43..31e87ed5b5614d411c5cad42723b23018f330144 100644 (file)
--- 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: