[squid-dev] [PATCH] Fix ICAP transactions that read a lot of data

[Alex Rousskov]

Hello,

    Trunk r13995 (Parser-NG: Convert the ICAP read buffer to an SBuf)
broke ICAP transactions that read a lot of data because the new
SBuf::consume() method often does not free buffer space, unlike the old
MemBuf::consume(). Trunk r14173 (Parser-NG: Transfer-Encoding:chunked
Parser) did not have a material effect on this bug AFAICT, even though
it replaced at least one new explicit consume() call with an SBuf
assignment. I think that just effectively moved the consume() call(s)
elsewhere.

Affected transactions fail with mayReadMore() exceptions because their
readBuf.spaceSize() is zero while they need to read more data.

Any append,parse,consume;append,parse,consume;... user of SBuf cannot
rely on SBuf::spaceSize() to be meaningful because even consuming the
entire SBuf contents may leave spaceSize() at zero! Instead such code
has to use SBuf::length() to keep buffer from growing too big and
SBuf::rawSpace(1) to ensure some space is available for reading when the
buffer is not too big.

The attached patch fixes this bug in code based on trunk r14083. Judging
by cache.log, the same patch has the right effect on the current trunk
(r14173) as well. However, the current trunk seems to be broken in other
(non-ICAP) places [related to large transaction handling?] so I am
unable to fully test the patch there right now.


I have not carefully checked whether other new SBuf::consume() users are
affected. If somebody has the time, I recommend analyzing all
SBuf::spaceSize() uses.


Thank you,

Alex.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: icap-ensure-space-t2.patch
Type: text/x-diff
Size: 5482 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150724/305466bc/attachment.patch>