Re: pack operation is thrashing my server
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 7, 2008, 17:41 UTC
- Message-ID
- <7vljy3n99c.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <alpine.LFD.1.10.0809071310030.23787@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
Show 28 quoted lines
> On Sat, 6 Sep 2008, Junio C Hamano wrote: > >> Linus Torvalds <torvalds@linux-foundation.org> writes: >> ... >> > Which it does - but the patch kind of violates that whole design. >> > >> > Now, it so happens that things seem to work, probably because the zlib >> > format does have enough synchronization in it to not try to continue past >> > the end _anyway_, but I think this makes the patch be of debatable value. >> >> I thought the fact we do check the status with Z_STREAM_END means that we >> do already expect and rely on zlib to know where the end of input stream >> is, and stop there (otherwise we say something fishy is going on and we >> error out), and it was part of the design, not just "so happens" and "has >> enough synch ... _anyway_". >> >> If input zlib stream were corrupted and it detected the end of stream too >> early, then check of "stream.total_out != size" would fail even though we >> would see "st == Z_STREAM_END". If input stream were corrupted and it >> went past the end marker, we will read past the end and into some garbage >> that is the in-pack header of the next object representation, but zlib >> shouldn't go berserk even in that case, and would stop after filling the >> slop you allocated in the buffer --- we would detect the situation from >> stream.total_out != size and most likely st != Z_STREAM_END in such a >> case. > > Unless I'm missing something, I think your analysis is right and > everything should be safe.
I obviously agree with you but what I forgot to mention in the above is that we also make sure stream.avail_in is set not to overrun the end of the current pack window (or the entire loose object data that is mmapped).