[OpenJDK 2D-Dev] Review request for JDK-6967419 : IndexOutOfBoundsException when drawing PNGs
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Tue Dec 1 19:12:52 UTC 2015
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 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
>
--
Best regards, Sergey.
More information about the 2d-dev
mailing list