[OpenJDK 2D-Dev] Review request for JDK-6967419 : IndexOutOfBoundsException when drawing PNGs

[Phil Race]

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 at 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 at openjdk.java.net
>     <mailto:2d-dev at 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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20151116/544512bd/attachment.html>