4206909 - adding Z_SYNC_FLUSH support to deflaters

[Martin Buchholz]

On Sat, Sep 5, 2009 at 00:23, Brandon Long<blong at google.com> wrote:
> On Fri, Sep 4, 2009 at 6:21 PM, Martin Buchholz <martinrb at google.com> wrote:
>> On Fri, Sep 4, 2009 at 18:02, Xueming Shen<Xueming.Shen at sun.com> wrote:
>>>> Overall, I'm least happy with #4, since I feel it leaves a bug.  flush()
>>>> on a stream should flush everything I've written to the stream.  This
>>>> bug is that it currently doesn't, and this doesn't fix it.  It makes it
>>>> possible for people to fix it (which isn't possible currently without
>>>> using a completely separate implementation), but it doesn't fix the bug.
>>
>> I think "bug" is going too far.  There are OutputStreams that can't dispose
>> of previously read data without reading some more first.  It depends on
>> the transformation.  DeflaterOutputStream is in a gray area -
>> it "could" write all the data to its underlying stream, but at the cost of
>> sacrificing some compression (which is its purpose).
>
> I think you're saying read when you mean write... and what is the
> point of calling flush if it doesn't mean to actually flush what I
> wrote?

I was thinking from the OutputStream's point of view - it "reads"
data when its "write" method is invoked.  I guess "receive" is a
better word.

Suppose you have a ByteSwappingOutputStream whose contract
was to swap every other pair of bytes.  If it has received an odd
number of bytes, its flush method cannot write the last byte,
because it needs another byte first.  You can interpret flush's
contract as saying to write all buffered output that is "ready"
to go out *unmodified* - it might not be expected that flush()
causes different bytes to be written just so that they can be
written *now*.  But we are proposing that DOS.flush()
does in fact modify the bytes written, possibly in a way that
violates DOS's contract, which is to compress the data.

Although DeflaterOutputStream's contract is not really clear -
is its contract to maximally compress the output, or merely to produce
output that can be read later by an Inflater?

That said, I still lean towards changing DOS.flush as suggested,
but I also understand why conservative maintainers might not.

> As for bug, how about :
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4077821
>
> and
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4032746

Those are some ancient bugs!
... that I don't recall having looked at in my
years of part-time maintenance of java.util.zip.

Martin

>
> Which both speak that it breaks the OutputStream interface (and both
> were closed as "not reproducible?)
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4206909
>
> The doc for Flushable.flush says:
>           Flushes this stream by writing any buffered output to the
> underlying stream.
>
> I think the lack of flush actually working on a DeflaterOutputStream
> is the bug that we're attempting to fix.