[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [8u] RFR JDK-8162461: Hang due to JNI up-call made whilst holding JNI critical
From: Ajit Ghaisas <ajit.ghaisas () oracle ! com>
Date: 2016-10-06 9:27:21
Message-ID: 0ff476a8-efb9-49e5-87c3-2fbe51623b0c () default
[Download RAW message or body]
Looks OK to me.
Regards,
Ajit
From: Philip Race
Sent: Wednesday, October 05, 2016 9:49 PM
To: Jayathirth D V
Cc: 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [8u] RFR JDK-8162461: Hang due to JNI up-call made \
whilst holding JNI critical lock.
+1
-phil.
On 9/29/16, 1:49 AM, Jayathirth D V wrote:
Hi Phil,
New changes are submitted in JDK9 under JDK-8166685.
Please find updated webrev for review in JDK8u:
HYPERLINK "http://cr.openjdk.java.net/%7Ejdv/8162461.8u/webrev.01/"http://cr.openjdk.java.net/~jdv/8162461.8u/webrev.01/ \
I have verified JCK, jtreg and JPRT for updated changes in JDK8u.
Thanks,
Jay
From: Philip Race
Sent: Friday, September 23, 2016 10:10 PM
To: Jayathirth D V
Cc: 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [8u] RFR JDK-8162461: Hang due to JNI up-call made \
whilst holding JNI critical lock.
You can't re-use the 9 bug ID in 9. So you will need to create a new bug to fix
that in 9.
But for 8u it can be part of this fix .. it would be weird to knowingly check in
a problem just so you could fix it separately.
-phil.
On 9/23/16, 12:13 AM, Jayathirth D V wrote:
Hi Phil,
I assumed setjmp/longjmp functions to be internal error handling mechanism rather \
than C standard library functions which provide non-local jumps.
I verified setjmp/longjmp usage in ImageIO JPEG context and we need that \
RELEASE_ARRAYS() call in writeImage() which I have removed.
Does I have to create a new Bug and add the RELEASE_ARRAY() call in writeImage() or I \
have to check-in this change under JDK-8162461 only?
After check-in of this modification I will raise new webrev for 8u backport.
Thanks,
Jay
From: Philip Race
Sent: Thursday, September 22, 2016 9:51 PM
To: Jayathirth D V
Cc: 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [8u] RFR JDK-8162461: Hang due to JNI up-call made \
whilst holding JNI critical lock.
I see this is mostly what I approved for JDK9 but I noticed you made a change
after I approved it and I did not see or approve the updated version.
I am concerned about this comment you posted for the JDK 9 webrev :
> Regarding "2856 RELEASE_ARRAYS(env, data, (const JOCTET \
> *)(dest->next_output_byte));". We don't need this RELEASE_ARRAY() call at this \
> place as we have not yet pinned any buffer. So I have removed it.
> Please find updated webrev for review :
> HYPERLINK "http://cr.openjdk.java.net/%7Ejdv/8162461/webrev.02/"http://cr.openjdk.java.net/~jdv/8162461/webrev.02/ \
>
What makes you so sure we have not pinned a buffer by then ?
setjmp is special. It may return from any point in the writing process.
In other words that removal looks wrong to me.
-phil.
On 9/15/16, 1:29 AM, Jayathirth D V wrote:
Hi,
Please review the following backport to JDK-8u from JDK-9 at your convenience:
JDK-9 review thread : \
http://mail.openjdk.java.net/pipermail/2d-dev/2016-September/007601.html
Bug : https://bugs.openjdk.java.net/browse/JDK-8162461
JDK-8u Webrev : HYPERLINK \
"http://cr.openjdk.java.net/%7Ejdv/8162461.8u/webrev.00/"http://cr.openjdk.java.net/~jdv/8162461.8u/webrev.00/ \
Issue : If we try to perform operations like reader.abort()/reader.dispose()/ \
reader.reset() in any of the IIOReadUpdateListener callbacks, JVM will throw deadlock \
error.
Root cause : We are making callbacks from native side to Java by holding some \
resources in JNI critical lock.
Solution : We have to release the JNI critical lock on the resources before we call \
Java function from native side. If we have JNI critical lock and we throw an \
Exception in that case also we should release the lock.
I have verified jtreg, JCK and JPRT in JDK8u-dev also.
Thanks,
Jay
[Attachment #3 (text/html)]
<html xmlns:v="urn:schemas-microsoft-com:vml" \
xmlns:o="urn:schemas-microsoft-com:office:office" \
xmlns:w="urn:schemas-microsoft-com:office:word" \
xmlns:x="urn:schemas-microsoft-com:office:excel" \
xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" \
xmlns="http://www.w3.org/TR/REC-html40"><head><META HTTP-EQUIV="Content-Type" \
CONTENT="text/html; charset=us-ascii"><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]--></head><body bgcolor=white lang=EN-US \
link="#0563C1" vlink="#954F72"><div class=WordSection1><p class=MsoNormal><span \
style='color:#1F497D'>Looks OK to me.<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'>Regards,<o:p></o:p></span></p><p class=MsoNormal><span \
style='color:#1F497D'>Ajit<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'> Philip Race <br><b>Sent:</b> Wednesday, October 05, 2016 \
9:49 PM<br><b>To:</b> Jayathirth D V<br><b>Cc:</b> 2d-dev<br><b>Subject:</b> Re: \
[OpenJDK 2D-Dev] [8u] RFR JDK-8162461: Hang due to JNI up-call made whilst holding \
JNI critical lock.<o:p></o:p></span></p></div></div><p \
class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>+1<br><br>-phil.<br><br>On \
9/29/16, 1:49 AM, Jayathirth D V wrote: <span \
style='font-size:12.0pt'><o:p></o:p></span></p><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'>New changes are submitted in JDK9 under \
JDK-8166685.</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'>Please find updated webrev for review in \
JDK8u:</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'><a \
href="http://cr.openjdk.java.net/%7Ejdv/8162461.8u/webrev.01/">http://cr.openjdk.java.net/~jdv/8162461.8u/webrev.01/</a> \
</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 verified JCK, jtreg and JPRT for updated changes in \
JDK8u.</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, September 23, 2016 \
10:10 PM<br><b>To:</b> Jayathirth D V<br><b>Cc:</b> 2d-dev<br><b>Subject:</b> Re: \
[OpenJDK 2D-Dev] [8u] RFR JDK-8162461: Hang due to JNI up-call made whilst holding \
JNI critical lock.</span><o:p></o:p></p></div></div><p \
class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>You can't re-use the 9 bug ID \
in 9. So you will need to create a new bug to fix<br>that in 9.<br><br>But for 8u it \
can be part of this fix .. it would be weird to knowingly check in<br>a problem just \
so you could fix it separately.<br><br><br>-phil.<br><br>On 9/23/16, 12:13 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:#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 assumed setjmp/longjmp functions to be internal error \
handling mechanism rather than C standard library functions which provide non-local \
jumps.</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>I verified \
setjmp/longjmp usage in ImageIO JPEG context and we need that RELEASE_ARRAYS() call \
in writeImage() which I have removed.</span><o:p></o:p></p><p class=MsoNormal><span \
style='color:#1F497D'>Does I have to create a new Bug and add the RELEASE_ARRAY() \
call in writeImage() or I have to check-in this change under JDK-8162461 only? \
</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'>After check-in of this modification I will raise new webrev for \
8u backport.</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><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> Thursday, September 22, 2016 \
9:51 PM<br><b>To:</b> Jayathirth D V<br><b>Cc:</b> 2d-dev<br><b>Subject:</b> Re: \
[OpenJDK 2D-Dev] [8u] RFR JDK-8162461: Hang due to JNI up-call made whilst holding \
JNI critical lock.</span><o:p></o:p></p></div></div><p \
class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>I see this is mostly what I \
approved for JDK9 but I noticed you made a change<br>after I approved it and I did \
not see or approve the updated version.<br>I am concerned about this comment you \
posted for the JDK 9 webrev :<br><br>>Regarding \
"2856 RELEASE_ARRAYS(env, data, \
(const JOCTET *)(dest->next_output_byte));". <br>>We don't need this \
RELEASE_ARRAY() call at this place as we have not yet pinned any buffer.<br>> So I \
have removed it.<br>>Please find updated webrev for review :<br>><a \
href="http://cr.openjdk.java.net/%7Ejdv/8162461/webrev.02/">http://cr.openjdk.java.net/~jdv/8162461/webrev.02/</a> \
<br><br>What makes you so sure we have not pinned a buffer by then ?<br>setjmp is \
special. It may return from any point in the writing process.<br><br>In other words \
that removal looks wrong to me.<br><br>-phil.<br><br>On 9/15/16, 1:29 AM, Jayathirth \
D V wrote: <o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p \
class=MsoNormal>Hi,<o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p \
class=MsoNormal>Please review the following backport to JDK-8u from JDK-9 at your \
convenience:<o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p \
class=MsoNormal>JDK-9 review thread : <a \
href="http://mail.openjdk.java.net/pipermail/2d-dev/2016-September/007601.html">http://mail.openjdk.java.net/pipermail/2d-dev/2016-September/007601.html</a> \
<o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>Bug : <a \
href="https://bugs.openjdk.java.net/browse/JDK-8162461">https://bugs.openjdk.java.net/browse/JDK-8162461</a> \
<o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>JDK-8u \
Webrev : <a href="http://cr.openjdk.java.net/%7Ejdv/8162461.8u/webrev.00/">http://cr.openjdk.java.net/~jdv/8162461.8u/webrev.00/</a> \
<o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>Issue : If \
we try to perform operations like reader.abort()/reader.dispose()/ reader.reset() in \
any of the IIOReadUpdateListener callbacks<span style='color:#1F497D'>, </span>JVM \
will throw deadlock error.<o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p \
class=MsoNormal>Root cause : We are making callbacks from native side to Java by \
holding some resources in JNI critical lock.<o:p></o:p></p><p \
class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>Solution : We have to release \
the JNI critical lock on the resources before we call Java function from native side. \
If we have JNI critical lock and we throw an Exception in that case also we should \
release the lock.<o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p \
class=MsoNormal>I have verified jtreg, JCK and JPRT in JDK8u-dev \
also.<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></blockquote></blockquote></div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic