Re: pack operation is thrashing my server

[Junio C Hamano <gitster@pobox.com>]

Nicolas Pitre <nico@cam.org> writes:

> 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).