Re: [Bug 2311] SQUID crashes/ restarts when ICAP enabled on respmod for HTTP body size greater than 100kb

On Fri, 2008-04-18 at 01:39 +0300, Tsantilas Christos wrote:
> Alex Rousskov wrote:
> > On Thu, 2008-04-17 at 11:54 -0600, Alex Rousskov wrote:
> >
> >> Short-term: surround the parsing call with try/catch. Handle the parsing
> >> exception by committing the buffer as if there was no error and aborting
> >> the transaction. Do it in v3.0 and v3.1. This should be OK because the
> >> existing parser should not leave the buffer in an inconsistent state.
> >>
> >> Long-term: Remove the above hack. Add code to BodyPipe to mark the
> >> internal buffer as "invalid". Any external access to such buffer would
> >> result in an exception being thrown. If the checkout object destructor
> >> catches an exception, it would invalidate the BodyPipe buffer rather
> >> than letting the exception escape. This would make the checkout process
> >> exception-safe even in the presence of an externally thrown exception.
> >
> I do not know about this Long-term solution.... The current squid-trunk
> BodyPipe code with some minor modifications (asserts to Must ) can
> handle this type of errors. This solution looks complicated and has the
> same result: an exception will be thrown but in a different position....

Assert-to-Must will not help you here because you cannot throw two
concurrent exceptions (see my earlier rant about that). The long-term
solution avoids the second exception. It invalidates the internal buffer
instead. I agree that it is rather complex; that's why I proposed a
simple alternative below :-).

> > Or a much simpler but a little more riskier alternative:
> >
> > Any-term: Change the semantics of the checkout interface. Make the code
> > getting access to the buffer responsible for keeping the buffer in a
> > consistent state at all times. Change ~BodyPipeCheckout action from
> > pipe.undoCheckOut to pipe.checkIn, with a level 2 warning.
> >
>
> This one looks the best solution. (In practice is equivalent with the
> short-term solution above)

Yes, except you do not need to try/catch anything.

> > As far as I know, current BodyPipe checkout users obey the rule
> > suggested above. These users are, essentially, StoreEntry::write and
> > ChunkedCodingParser::parse.
> Looks that it is OK, but I will look closer tomorrow ......

Thank you,

Alex.
Received on Tue Apr 22 2008 - 13:24:33 MDT