Author: Alex Rousskov <rousskov@measurement-factory.com>

Bug 2311: crashes with ICAP RESPMOD for HTTP body size greater than 100kb

BodyPipe::undoCheckOut() must not assert that undo is possible because it
is not (currently) possible if the pipe buffer was modified.

BodyPipe::undoCheckOut() must not throw if undo is not possible because it is
often called when there is already an exception thrown and because it is
called from the BodyPipeCheckout destructor and destructors should not throw
(this case is an illustration for one reason why they should not).

Currently, we only use an implicit undo, and only when an exception
is being thrown while the buffer is checked out.

Currently, the code that does checkout is probably safe because it should
terminate the transaction if a parser throws. However, this is not 100%
guaranteed, and the situation may change without us noticing.

TODO: consider implementing the long-term solution discussed at
http://www.mail-archive.com/squid-dev@squid-cache.org/msg07910.html
COW-buffers may help here as well

diff --git a/src/BodyPipe.cc b/src/BodyPipe.cc
index 98b979e24c364164d5f199dcf7b46cdffc20f62a..799f81b49952960cd790ea2405e04bfabba7ee1a 100644 (file)
--- a/src/BodyPipe.cc
+++ b/src/BodyPipe.cc
@@ -337,7 +337,7 @@ BodyPipe::undoCheckOut(Checkout &checkout)
     // raw buffers should always check them in (possibly unchanged)
     // instead of relying on the automated undo mechanism of Checkout.
     // The code can always use a temporary buffer to accomplish that.
-    assert(checkout.checkedOutSize == currentSize);
+    Must(checkout.checkedOutSize == currentSize);
 }
 
 // TODO: Optimize: inform consumer/producer about more data/space only if
@@ -454,8 +454,13 @@ BodyPipeCheckout::BodyPipeCheckout(BodyPipe &aPipe): pipe(aPipe),
 
 BodyPipeCheckout::~BodyPipeCheckout()
 {
-    if (!checkedIn)
-        pipe.undoCheckOut(*this);
+    if (!checkedIn) {
+        // Do not pipe.undoCheckOut(*this) because it asserts or throws
+        // TODO: consider implementing the long-term solution discussed at
+        // http://www.mail-archive.com/squid-dev@squid-cache.org/msg07910.html
+        debugs(91,2, HERE << "Warning: cannot undo BodyPipeCheckout");
+        pipe.checkIn(*this);
+    }
 }
 
 void