]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Bug #2311 fix: crashes with ICAP RESPMOD for HTTP body size greater than 100kb
authorAlex Rousskov <rousskov@measurement-factory.com>
Sun, 5 Sep 2010 23:34:08 +0000 (17:34 -0600)
committerAlex Rousskov <rousskov@measurement-factory.com>
Sun, 5 Sep 2010 23:34:08 +0000 (17:34 -0600)
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.

src/BodyPipe.cc

index 4b8b6bd261675bd0b8c589284e98d9f7ac1cff1d..42c0f9e2c7cb38868caf473fb9d9915ee056b952 100644 (file)
@@ -346,7 +346,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
@@ -463,8 +463,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