Hi Sergey, Thanks for your suggestion. I verified all other image writers with present code(For WBMP after changing BufferedImageType to TYPE_BYTE_BINARY) there are no problems in them.
Cache is closed properly and we don't see IndexOutOfBoundsException. Thanks, Jay -----Original Message----- From: Sergey Bylokhov Sent: Wednesday, December 02, 2015 12:43 AM To: Jayathirth D V; Philip Race Cc: 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] Review request for JDK-6967419 : IndexOutOfBoundsException when drawing PNGs 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.
