Bug 3153: Prevent ICAP RESPMOD transactions getting stuck with the adapted body.

Part 1.

  Server is expected to receive adapted response headers and then consume the
  adapted response body, if any. If the server receives the headers and then
  aborts, it must notify the ICAP side that nobody will consume the body.
  Otherwise, the ICAP transaction will fill the BodyPipe buffer and get stuck
  waiting for the consumer to free some space.

Part 2:

  This fix still leaves one potential race condition unhandled: The ICAP
  Initiatee disappears right after sending the adapted headers to the Server
  (because there is nothing else for that initiatee to do). After the
  noteAdaptationAnswer() call is scheduled by ICAP and before it is received by
  the Server job, there is no usable link between Server and ICAP.  There is no
  way for the Server to notify the ICAP transaction that the Server job is
  aborting during that time (and there is no Server job at all after it aborts,
  naturally).

  The solutions is to develop a custom AsyncCall which will call the
  expectNoConsumption() on the message pipe if the call cannot be dialed (i.e.,
  the message cannot be delivered to Server).

diff --git a/src/Server.cc b/src/Server.cc
index cdc2a2da53e1391c0e18b69eec063b92ce3bf6df..8cd2678f07db59d1f99e4722c8df60a2bf6df536 100644 (file)
--- a/src/Server.cc
+++ b/src/Server.cc
@@ -648,8 +648,17 @@ ServerStateData::noteAdaptationAnswer(HttpMsg *msg)
 {
     clearAdaptation(adaptedHeadSource); // we do not expect more messages
 
-    if (abortOnBadEntry("entry went bad while waiting for adapted headers"))
+    if (abortOnBadEntry("entry went bad while waiting for adapted headers")) {
+        // If the adapted response has a body, the ICAP side needs to know
+        // that nobody will consume that body. We will be destroyed upon
+        // return. Tell the ICAP side that it is on its own.
+        HttpReply *rep = dynamic_cast<HttpReply*>(msg);
+        assert(rep);
+        if (rep->body_pipe != NULL)
+            rep->body_pipe->expectNoConsumption();
+
         return;
+    }
 
     HttpReply *rep = dynamic_cast<HttpReply*>(msg);
     assert(rep);

diff --git a/src/adaptation/Initiate.cc b/src/adaptation/Initiate.cc
index ffba5bc923b7b64d4f5d5c8eef141c6db373269f..83634b85f3f9b15f8ae0ae9d9e1f4343f467befc 100644 (file)
--- a/src/adaptation/Initiate.cc
+++ b/src/adaptation/Initiate.cc
@@ -27,6 +27,22 @@ private:
     AnswerDialer &operator =(const AnswerDialer &); // not implemented
 };
 
+/// Calls expectNoConsumption() if noteAdaptationAnswer async call is
+/// scheduled but never fired (e.g., because the HTTP transaction aborts).
+class AnswerCall: public AsyncCallT<AnswerDialer>{
+    AnswerCall(const char *aName, const AnswerDialer &aDialer) :
+        AsyncCallT<AnswerDialer>(93, 5, aName, aDialer), fired(false) {}
+    virtual void fire() {
+        fired = true; 
+        AsyncCallT<AnswerDialer>::fire();
+    }
+    virtual ~AnswerCall() {
+        if (!fired && dialer.arg1.message != NULL && dialer.arg1.message->body_pipe != NULL)
+            dialer.arg1.message->body_pipe->expectNoConsumption();
+    }
+    bool fired; ///< whether we fired the call
+};
+
 } // namespace Adaptation
 
 
@@ -73,8 +89,9 @@ void Adaptation::Initiate::clearInitiator()
 void Adaptation::Initiate::sendAnswer(HttpMsg *msg)
 {
     assert(msg);
-    CallJob(93, 5, __FILE__, __LINE__, "Initiator::noteAdaptationAnswer",
-            AnswerDialer(theInitiator, &Initiator::noteAdaptationAnswer, msg));
+    AsyncCall::Pointer call = new AnswerCall("Initiator::noteAdaptationAnswer",
+                                             AnswerDialer(theInitiator, &Initiator::noteAdaptationAnswer, answer));
+    ScheduleCallHere(call);
     clearInitiator();
 }