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.
