[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-runtime-dev
Subject: Re: RFR (S) 8016075 - Win32 crash with CDS enabled and small heap size
From: Christian Thalinger <christian.thalinger () oracle ! com>
Date: 2013-06-27 18:07:33
Message-ID: AA22DE34-6E1E-4BE2-94A8-525210ED2CED () oracle ! com
[Download RAW message or body]
On Jun 27, 2013, at 9:19 AM, Ioi Lam <ioi.lam@oracle.com> wrote:
> Chris,
>
> is it OK for me to send you a patch instead? Currently we are not calling this \
> function during dump time and I don't want to commit any code that has not been \
> tested.
Sure. This was more a general question than a review comment. Sorry, I should have \
made this clear.
-- Chris
>
> Harold,
>
> Thanks for the comment. I will modify it as you said.
>
> - Ioi
>
>
> On 06/27/2013 09:02 AM, Christian Thalinger wrote:
> >
> > On Jun 27, 2013, at 6:42 AM, harold seigel <harold.seigel@oracle.com> wrote:
> >
> > > Hi Ioi,
> > >
> > > The changes look good.
> > >
> > > You can simplify MetaspaceShared::is_in_shared_space(...) to something like:
> > > // Return true if given address is in the mapped shared space.
> > > bool MetaspaceShared::is_in_shared_space(const void* p) {
> > > return UseSharedSpaces && FileMapInfo::current_info()->is_in_shared_space(p);
> > > }
> >
> > Would it be possible to change that method to also return a valid answer when \
> > dumping shared spaces? I'd need this for something else I'm working on.
> > -- Chris
> > >
> > > Thanks, Harold
> > >
> > > On 6/27/2013 12:10 AM, Ioi Lam wrote:
> > > > Please review a small fix:
> > > >
> > > > http://cr.openjdk.java.net/~iklam/8016075/cds_is_shared_crash_003/
> > > >
> > > > Bug: Win32 crash with CDS enabled and small heap size
> > > >
> > > > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8016075
> > > > https://jbs.oracle.com/bugs/browse/JDK-8016075
> > > >
> > > > Summary of fix:
> > > >
> > > > The function MetaspaceShared::is_in_shared_space() was testing with
> > > > incorrect bounds. As a resullt, a dynamically loaded InstanceKlass
> > > > was incorrectly identified as an InstanceKlass stored in the CDS
> > > > archive. This caused the following code to fail:
> > > >
> > > > bool InstanceKlass::link_class_impl(...) {
> > > > ...
> > > > if (!this_oop()->is_shared()) {
> > > > ResourceMark rm(THREAD);
> > > > this_oop->vtable()->initialize_vtable(true, CHECK_false); //<< failed to \
> > > > execute this_oop->itable()->initialize_itable(true, CHECK_false);
> > > > }
> > > >
> > > > Hence, Method::vtable_index() could return an incorrect value for some
> > > > Methods, leading to various mysterious crashes.
> > > >
> > > > The fix is for MetaspaceShared::is_in_shared_space() to consider the actual \
> > > > used spaces in the CDS archive regions.
> > > >
> > > > Tests:
> > > >
> > > > JPRT
> > > > UTE (vm.runtime.testlist, vm.quick.testlist, \
> > > > vm.parallel_class_loading.testlist)
> > > > Thanks
> > > > - Ioi
> > > >
> > > >
> > > >
> > >
> >
>
[Attachment #3 (unknown)]
<html><head><meta http-equiv="Content-Type" content="text/html \
charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: \
space; -webkit-line-break: after-white-space; "><br><div><div>On Jun 27, 2013, at \
9:19 AM, Ioi Lam <<a href="mailto:ioi.lam@oracle.com">ioi.lam@oracle.com</a>> \
wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type">
<div bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Chris, <br>
<br>
is it OK for me to send you a patch instead? Currently we are not
calling this function during dump time and I don't want to commit
any code that has not been \
tested.<br></div></div></blockquote><div><br></div>Sure. This was more a \
general question than a review comment. Sorry, I should have made this \
clear.</div><div><br></div><div>-- Chris</div><div><br><blockquote type="cite"><div \
bgcolor="#FFFFFF" text="#000000"><div class="moz-cite-prefix"> <br>
Harold,<br>
<br>
Thanks for the comment. I will modify it as you said.<br>
<br>
- Ioi<br>
<br>
<br>
On 06/27/2013 09:02 AM, Christian Thalinger wrote:<br>
</div>
<blockquote cite="mid:6C52DC68-7024-496B-AE50-A6707D91DE19@oracle.com" \
type="cite"> <meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
<br>
<div>
<div>On Jun 27, 2013, at 6:42 AM, harold seigel <<a moz-do-not-send="true" \
href="mailto:harold.seigel@oracle.com">harold.seigel@oracle.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type">
<div text="#000000" bgcolor="#FFFFFF"> Hi Ioi,<br>
<br>
The changes look good. <br>
<br>
You can simplify MetaspaceShared::is_in_shared_space(...) to
something like:<br>
<blockquote>// Return true if given address is in the mapped
shared space.<br>
bool MetaspaceShared::is_in_shared_space(const void* p) {<br>
return UseSharedSpaces &&
FileMapInfo::current_info()->is_in_shared_space(p);<br>
}<br>
</blockquote>
</div>
</blockquote>
<div><br>
</div>
Would it be possible to change that method to also return a
valid answer when dumping shared spaces? I'd need this for
something else I'm working on.</div>
<div><br>
</div>
<div>-- Chris<br>
<blockquote type="cite">
<div text="#000000" bgcolor="#FFFFFF">
<blockquote> </blockquote>
<br>
Thanks, Harold<br>
<br>
<div class="moz-cite-prefix">On 6/27/2013 12:10 AM, Ioi Lam
wrote:<br>
</div>
<blockquote cite="mid:51CBBB4A.70308@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<code>Please review a small fix:</code><br>
<div class="moz-forward-container"><code> </code><code><br>
</code><code><tt> <a moz-do-not-send="true" \
class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/%7Eiklam/8016075/cds_is_shared_crash_003/">http://cr. \
openjdk.java.net/~iklam/8016075/cds_is_shared_crash_003/</a></tt></code><code><br> \
</code><code><br> </code><code>Bug: Win32 crash with CDS enabled and small
heap size</code><code><br>
</code><code><br>
</code><code><tt> <a moz-do-not-send="true" \
class="moz-txt-link-freetext" \
href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8016075">http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8016075</a></tt></code><code><br>
</code><code><tt> <a moz-do-not-send="true" \
class="moz-txt-link-freetext" \
href="https://jbs.oracle.com/bugs/browse/JDK-8016075">https://jbs.oracle.com/bugs/browse/JDK-8016075</a></tt></code><code><br>
</code><code><br>
</code><code>Summary of fix:</code><code><br>
</code><code><br>
</code><code> The function
MetaspaceShared::is_in_shared_space() was testing \
with</code><code><br>
</code><code> incorrect bounds. As a resullt, a
dynamically loaded InstanceKlass</code><code><br>
</code><code> was incorrectly identified as an
InstanceKlass stored in the CDS</code><code><br>
</code><code> archive. This caused the following \
code to fail:</code><code><br>
</code><code><br>
</code><code> bool
InstanceKlass::link_class_impl(...) {</code><code><br>
</code><code> ...</code><code><br>
</code><code> if \
(!this_oop()->is_shared()) {</code><code><br>
</code><code> ResourceMark \
rm(THREAD);</code><code><br> </code><code>
this_oop->vtable()->initialize_vtable(true,
CHECK_false); //<< failed to execute</code><code><br>
</code><code>
this_oop->itable()->initialize_itable(true,
CHECK_false);</code><code><br>
</code><code> }</code><code><br>
</code><code><br>
</code><code> Hence, Method::vtable_index() could
return an incorrect value for some</code><code><br>
</code><code> Methods, leading to various \
mysterious crashes.</code><code><br>
</code><code><br>
</code><code> The fix is for
MetaspaceShared::is_in_shared_space() to consider the
actual used</code><code><br>
</code><code> spaces in the CDS archive \
regions.</code><code><br> </code><code><br>
</code><code>Tests:</code><code><br>
</code><code><br>
</code><code> JPRT</code><code><br>
</code><code> UTE (vm.runtime.testlist,
vm.quick.testlist, \
vm.parallel_class_loading.testlist)</code><code><br> </code><code><br>
</code><code>Thanks</code><code><br>
</code><code>- Ioi</code><code><br>
</code><code><br>
</code> <br>
</div>
<br>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br>
</blockquote>
<br>
</div>
</blockquote></div><br></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic