[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] Review request for JDK-6967419 : IndexOutOfBoundsException when drawing PNGs
From: Phil Race <philip.race () oracle ! com>
Date: 2015-11-23 18:47:50
Message-ID: 56535F56.7010906 () oracle ! com
[Download RAW message or body]
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@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
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">So you should re-open 8041746 and close
it as a dup of 6967419.<br>
<br>
I am approving 6967419 since it fixes an apparent problem.<br>
<br>
There may be additional problems but these are still theoretical
..<br>
<br>
-phil.<br>
<br>
On 11/17/2015 01:11 AM, Jayathirth D V wrote:<br>
</div>
<blockquote cite="mid:73963f44-6f4b-41e0-a0a8-37e0a7ef9d4d@default"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
<meta name="Generator" content="Microsoft Word 15 (filtered
medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
color:black;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle18
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle19
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
<div class="WordSection1">
<p class="MsoNormal"><span style="color:#1F497D">Hi \
Phil,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Thanks for
pointing to JDK-8041746 .<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><u><span style="color:#1F497D">My
observations:<o:p></o:p></span></u></p>
<p class="MsoNormal" style="margin-left:.5in"><span
style="color:#1F497D">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).<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal" style="text-indent:.5in"><span
style="color:#1F497D">This might explain why submitter is
seeing 7 out of 20 fail.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal" style="margin-left:.5in"><span
style="color:#1F497D">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Please let us
know your inputs.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span \
style="color:#1F497D">Thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Jay<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #E1E1E1
1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span \
style="color:windowtext">From:</span></b><span style="color:windowtext"> Phil Race \
<br> <b>Sent:</b> Tuesday, November 17, 2015 3:22 AM<br>
<b>To:</b> Jayathirth D V<br>
<b>Cc:</b> Prasanta Sadhukhan; <a class="moz-txt-link-abbreviated" \
href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br> \
<b>Subject:</b> Re: Review request for JDK-6967419 :
IndexOutOfBoundsException when drawing PNGs<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">This one reads like it should be obvious
but I find it less so ..<br>
The unsatisfying part is that we do not seem to know what
caused<br>
the IOException in the customer case.<br>
<br>
Andrew came up with a way to reproduce the symptoms but we
really<br>
don't know what caused the exception in the case of the
submitter.<br>
It does not seem likely he was 'deliberately' throwing an
exception to mess up his own application.<br>
<br>
I just found this : <a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8041746">https://bugs.openjdk.java.net/browse/JDK-8041746</a><br>
<br>
The interesting part is that this bug (the one you are
working on)<br>
the submitter also wrote that he was using "a
ServletOutputStream"<br>
<br>
So consequently I wonder if it was something like what is
described in<br>
8041746 is going on here. It could explain how he sees 7 out
of 20 fail.<br>
<br>
Please take a look at that one to have a think about it.<br>
Would your fix help that real world case ?<br>
<br>
-phil.<br>
<br>
On 11/12/2015 08:11 PM, Jayathirth D V wrote:<span
style="font-size:12.0pt"><o:p></o:p></span></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="color:#1F497D">Hi \
Phil,</span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:#1F497D"> </span><o:p></o:p></p> <p class="MsoNormal"><span \
style="color:#1F497D">I have added
public evaluation in bug. Please review.</span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:#1F497D">Thanks,</span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:#1F497D">Jay</span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:#1F497D"> </span><o:p></o:p></p> <div>
<div style="border:none;border-top:solid #E1E1E1
1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span \
style="color:windowtext">From:</span></b><span style="color:windowtext"> Philip Race \
<br> <b>Sent:</b> Friday, November 13, 2015 12:11 AM<br>
<b>To:</b> Jayathirth D V<br>
<b>Cc:</b> Prasanta Sadhukhan; <a
moz-do-not-send="true"
href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br>
<b>Subject:</b> Re: Review request for JDK-6967419 :
IndexOutOfBoundsException when drawing PNGs</span><o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">Please add a *public* evaluation to the
bug report. I will look at it more then ..<br>
<br>
-phil.<br>
<br>
On 11/6/15, 2:20 AM, Jayathirth D V wrote: <o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="color:windowtext">Hi
Prasanta,</span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:windowtext"> </span><o:p></o:p></p> <p class="MsoNormal"><span \
style="color:windowtext">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.</span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:windowtext"> </span><o:p></o:p></p> <p class="MsoNormal"><span \
style="color:windowtext">And I have modified test to not catch IOBE as per your
suggestion. Please find updated Webrev link:</span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:windowtext"> </span><o:p></o:p></p> <p class="MsoNormal"><span \
style="color:windowtext"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Erchamyal/jay/6967419/webrev.01/"><span
style="color:windowtext">http://cr.openjdk.java.net/~rchamyal/jay/6967419/webrev.01/</span></a></span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:windowtext"> </span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:windowtext">Thanks,</span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:windowtext">Jay</span><o:p></o:p></p>
<p class="MsoNormal"><span \
style="color:#1F497D"> </span><o:p></o:p></p> <div>
<div style="border:none;border-top:solid #E1E1E1
1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span \
style="color:windowtext">From:</span></b><span style="color:windowtext"> prasanta \
sadhukhan <br> <b>Sent:</b> Friday, November 06, 2015 2:45 PM<br>
<b>To:</b> Jayathirth D V; <a
moz-do-not-send="true"
\
href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br> <b>Cc:</b> \
Philip Race<br> <b>Subject:</b> Re: Review request for JDK-6967419 :
IndexOutOfBoundsException when drawing PNGs</span><o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">Hi Jay,<br>
<br>
looks ok but<br>
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().<br>
Also, I guess you should not consume IOB Exception and let
it be thrown to user instead of RuntimeException after
catching IOBE.<br>
<br>
Regards<br>
Prasanta<o:p></o:p></p>
<div>
<p class="MsoNormal">On 11/5/2015 5:25 PM, Jayathirth D V
wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal">Hello All,<o:p></o:p></p>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">Please review following fix in \
jdk9:<o:p></o:p></p> <p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">Bug : <a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-6967419">https://bugs.openjdk.java.net/browse/JDK-6967419</a><o:p></o:p></p>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">Webrev : <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Erchamyal/jay/6967419/webrev.00/">http://cr.openjdk.java.net/~rchamyal/jay/6967419/webrev.00/</a><o:p></o:p></p>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">Bug : IndexOutOfBoundsException when
drawing PNGs<o:p></o:p></p>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">Root cause : When user intentionally
throws IO Exception while write is happening. <br>
&nbs \
p; 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. <br>
&nbs \
p; So \
in finishChunk(), startPos will be less than flushedPos. This is causing
IndexOutOfBoundException in stream.seek() and cache is
not closed.<o:p></o:p></p>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">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.<o:p></o:p></p> <p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
<p class="MsoNormal">Jay<o:p></o:p></p>
<p class="MsoNormal"> <o:p></o:p></p>
</blockquote>
<p class="MsoNormal"><span
style="font-size:12.0pt;font-family:"Times New
Roman",serif"> </span><o:p></o:p></p>
</blockquote>
</blockquote>
<p class="MsoNormal"><span
style="font-size:12.0pt;font-family:"Times New
Roman",serif"><o:p> </o:p></span></p>
</div>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic