Hi, Jay.
Can you please check other writers and confirm that similar issues are not exists there(just try a different writers in the test)? If the problem exists it can be fixed as a separate issue, if everything works as expected nothing should be changed.

On 17.11.15 14:41, Jayathirth D V wrote:
Hi Phil,

Thanks for pointing to JDK-8041746 .

_My observations:_

I think with Andrew’s test case we are able to identify the problem
submitter might have faced in JDK-6967419 . By deliberately throwing
exception at count 30000L we are trying to reproduce scenario of
possible IOException where client is closing the socket(probably because
of communication problem between client & server) and IDAT chunk is
being written. If we change count to any other number(like 10L) which
relates to writing of data apart from IDAT chunk we don’t see any
issue(no exception and cache is closed properly).

This might explain why submitter is seeing 7 out of 20 fail.

Also by using the test case we are able to see flushedPos going past by
4 bytes to Pos when ios.close() is called as mentioned by submitter in
JDK-6967419. So catching the IOException and updating ‘startPos’ value,
will not result in IndexOutOfBoundsException and ios.close() will be
performed properly.

Please let us know your inputs.

Thanks,

Jay

*From:*Phil Race
*Sent:* Tuesday, November 17, 2015 3:22 AM
*To:* Jayathirth D V
*Cc:* Prasanta Sadhukhan; 2d-dev@openjdk.java.net
*Subject:* Re: Review request for JDK-6967419 :
IndexOutOfBoundsException when drawing PNGs

This one reads like it should be obvious but I find it less so ..
The unsatisfying part is that we do not seem to know what caused
the IOException in the customer case.

Andrew came up with a way to reproduce the symptoms but we really
don't know what caused the exception in the case of the submitter.
It does not seem likely he was 'deliberately' throwing an exception to
mess up his own application.

I just found this : https://bugs.openjdk.java.net/browse/JDK-8041746

The interesting part is that this bug (the one you are working on)
the submitter also wrote that he was using "a ServletOutputStream"

So consequently I wonder if it was something like what is described in
8041746 is going on here. It could explain how he sees 7 out of 20 fail.

Please take a look at that one to have a think about it.
Would your fix help that real world case ?

-phil.

On 11/12/2015 08:11 PM, Jayathirth D V wrote:

    Hi Phil,

    I have added public evaluation in bug. Please review.

    Thanks,

    Jay

    *From:*Philip Race
    *Sent:* Friday, November 13, 2015 12:11 AM
    *To:* Jayathirth D V
    *Cc:* Prasanta Sadhukhan; 2d-dev@openjdk.java.net
    <mailto:2d-dev@openjdk.java.net>
    *Subject:* Re: Review request for JDK-6967419 :
    IndexOutOfBoundsException when drawing PNGs

    Please add a *public* evaluation to the bug report. I will look at
    it more then ..

    -phil.

    On 11/6/15, 2:20 AM, Jayathirth D V wrote:

        Hi Prasanta,

        As discussed, only in case of write_IDAT there is finally block
        which calls ios.finish() which internally calls seek() with
        improper startPos. In other cases we are not trying to access
        improper startPos because there is no call to ios.finish(). We
        can verify this behavior by changing logic where we throw
        IOException in test case.

        And I have modified test to not catch IOBE as per your
        suggestion. Please find updated Webrev link:

        http://cr.openjdk.java.net/~rchamyal/jay/6967419/webrev.01/
        <http://cr.openjdk.java.net/%7Erchamyal/jay/6967419/webrev.01/>

        Thanks,

        Jay

        *From:*prasanta sadhukhan
        *Sent:* Friday, November 06, 2015 2:45 PM
        *To:* Jayathirth D V; 2d-dev@openjdk.java.net
        <mailto:2d-dev@openjdk.java.net>
        *Cc:* Philip Race
        *Subject:* Re: Review request for JDK-6967419 :
        IndexOutOfBoundsException when drawing PNGs

        Hi Jay,

        looks ok but
        I guess you need to do the same for finish() method too in
        similar way you did for finishChunk() as finish() is called from
        write_IHDR, write_CHRM etc and it calls flushBefore().
        Also, I guess you should not consume IOB Exception and let it be
        thrown to user instead of RuntimeException after catching IOBE.

        Regards
        Prasanta

        On 11/5/2015 5:25 PM, Jayathirth D V wrote:

            Hello All,

            Please review following fix in jdk9:

            Bug : https://bugs.openjdk.java.net/browse/JDK-6967419

            Webrev :
            http://cr.openjdk.java.net/~rchamyal/jay/6967419/webrev.00/
            <http://cr.openjdk.java.net/%7Erchamyal/jay/6967419/webrev.00/>

            Bug : IndexOutOfBoundsException when drawing PNGs

            Root cause : When user intentionally throws IO Exception
            while write is happening.
                                       We call ios.finish() in finally
            block of write_IDAT() which internally goes to
            finishChunk(). But the startPos of the chunk is still
            pointing to present IDAT chunk but flushedPos(streamPos) is
            pointing to end of  IDAT chunk.
                                       So in finishChunk(), startPos
            will be less than flushedPos. This is causing
            IndexOutOfBoundException in stream.seek() and cache is not
            closed.

            Solution : If IOException is thrown by user, catch the
            exception while write is happening and update startPos to
            streamPos. So that when seek() happens in finishChunk() we
            don’t see IndexOutOfBoundsException and cache is closed
            properly.

            Thanks,

            Jay



--
Best regards, Sergey.

Reply via email to