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

Phil Race philip.race at oracle.com
Mon Nov 23 18:47:50 UTC 2015


So you should re-open 8041746 and close it as a dup of 6967419.

I am approving 6967419 since it fixes an apparent problem.

There may be additional problems but these are still theoretical ..

-phil.

On 11/17/2015 01:11 AM, 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 at 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 at openjdk.java.net
>     <mailto: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/20151123/1f9b1d05/attachment.html>


More information about the 2d-dev mailing list