Partial bug #2964 fix: Avoid stuck ICAP REQMOD transactions when HTTP fails.

Added doneWithRetries() and used it to inform the request body sender that
there will be no more body consumers. This allows the sender (e.g., an ICAP
REQMOD transaction) to quit instead of waiting for body buffer space forever.

Moved !self check into checkRetry() because we need to call doneWithRetries()
even if self is nil. The move should not affect the old code.

Based on lp 3p1-rock branch, r9610.

diff --git a/src/forward.cc b/src/forward.cc
index 41f1fabb4d04d3960de07649d6ea95395be83a92..6f366785f7fbc0de354ee71dbb84b3406b1cf4a5 100644 (file)
--- a/src/forward.cc
+++ b/src/forward.cc
@@ -165,6 +165,8 @@ FwdState::~FwdState()
 
     serversFree(&servers);
 
+    doneWithRetries();
+
     HTTPMSGUNLOCK(request);
 
     if (err)
@@ -415,6 +417,12 @@ FwdState::checkRetry()
     if (shutting_down)
         return false;
 
+    if (!self) { // we have aborted before the server called us back
+        debugs(17, 5, HERE << "not retrying because of earlier abort");
+        // we will be destroyed when the server clears its Pointer to us
+        return false;
+    }
+
     if (entry->store_status != STORE_PENDING)
         return false;
 
@@ -498,12 +506,6 @@ FwdState::serverClosed(int fd)
 void
 FwdState::retryOrBail()
 {
-    if (!self) { // we have aborted before the server called us back
-        debugs(17, 5, HERE << "not retrying because of earlier abort");
-        // we will be destroyed when the server clears its Pointer to us
-        return;
-    }
-
     if (checkRetry()) {
         int originserver = (servers->_peer == NULL);
         debugs(17, 3, "fwdServerClosed: re-forwarding (" << n_tries << " tries, " << (squid_curtime - start_t) << " secs)");
@@ -540,13 +542,26 @@ FwdState::retryOrBail()
         return;
     }
 
-    if (!err && shutting_down) {
+    // TODO: should we call completed() here and move doneWithRetries there?
+    doneWithRetries();
+
+    if (self != NULL && !err && shutting_down) {
         errorCon(ERR_SHUTTING_DOWN, HTTP_SERVICE_UNAVAILABLE, request);
     }
 
     self = NULL;       // refcounted
 }
 
+// If the Server quits before nibbling at the request body, the body sender
+// will not know (so that we can retry). Call this if we will not retry. We
+// will notify the sender so that it does not get stuck waiting for space.
+void
+FwdState::doneWithRetries()
+{
+    if (request && request->body_pipe != NULL)
+        request->body_pipe->expectNoConsumption();
+}
+
 // called by the server that failed after calling unregister()
 void
 FwdState::handleUnregisteredServerEnd()

diff --git a/src/forward.h b/src/forward.h
index 0ac5ae90c6ac841cdaf2ab8c262bb8e5df7ac0ff..26214b3a202756afe1b736ebba60be6d0ced00ea 100644 (file)
--- a/src/forward.h
+++ b/src/forward.h
@@ -62,6 +62,7 @@ private:
 
     static void logReplyStatus(int tries, http_status status);
     void updateHierarchyInfo();
+    void doneWithRetries();
     void completed();
     void retryOrBail();
     static void RegisterWithCacheManager(void);