[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: (11) RFR (S) JDK-8205509: assert(_name_and_id != 0LL) failed: encountered a class loader null na
From:       Lois Foltan <lois.foltan () oracle ! com>
Date:       2018-06-25 14:13:51
Message-ID: eb2334bb-ea84-79cc-6225-cdd503b47f3d () oracle ! com
[Download RAW message or body]

Thanks Coleen!
Lois

On 6/25/2018 10:12 AM, coleen.phillimore@oracle.com wrote:
> 
> 
> Lois, This looks good!
> Thanks,
> Coleen
> 
> On 6/22/18 7:39 PM, Lois Foltan wrote:
> > On 6/22/2018 7:28 PM, coleen.phillimore@oracle.com wrote:
> > 
> > > 
> > > Lois,
> > > This looks good!
> > > 
> > > http://cr.openjdk.java.net/~lfoltan/bug_jdk8205509/webrev/src/hotspot/share/classfile/classLoaderData.cpp.udiff.html \
> > >  
> > > 
> > > if (!h_class_loader.is_null()) {
> > > _class_loader = _handles.add(h_class_loader());
> > > }
> > > 
> > > + if (!h_class_loader.is_null()) {
> > > + _class_loader_klass = h_class_loader->klass();
> > > + }
> > > +
> > > 
> > > Can you put the _class_loader_klass under the if statement above it?
> > > 
> > > + } else {
> > > + return _class_loader_klass->external_name();
> > > 
> > > Can you add short comment:   // May be called in a race before 
> > > _name_and_id is initialized.
> > > 
> > > Unfortunately, there isn't a good way to assert this (other than 
> > > maybe a flag which would serve the same purpose as a null 
> > > _name_and_id), and this code path is sensitive to safepoints because 
> > > the link from class_loader to ClassLoaderData and the addition of 
> > > ClassLoaderData to the CLDG needs to appear atomic.
> > Hi Coleen,
> > 
> > Thanks for the review, good comments.   They all have been addressed 
> > and more testing is in progress.   See updated webrev at:
> > 
> > http://cr.openjdk.java.net/~lfoltan/bug_jdk8205509.1/webrev/
> > 
> > Lois
> > 
> > > 
> > > Thanks for the speedy fix.
> > > Coleen
> > > 
> > > On 6/22/18 7:00 PM, Lois Foltan wrote:
> > > > Please review the following change to address a hs-tier5 test 
> > > > failure. The recent change for JDK-8202605 aggravated an existing 
> > > > race condition within ClassLoaderDataGraph::add() where a given 
> > > > class loader's ClassLoaderData structure is added to the 
> > > > ClassLoaderDataGraph prior to full initialization of the CLD's 
> > > > _class_loader_klass, _name and _name_and_id fields within 
> > > > ClassLoaderData::initialize_name_and_klass.   To ensure that methods 
> > > > loader_name() and loader_name_and_id() always provide a string for 
> > > > the class loader name, the fix involved moving the field, 
> > > > _class_loader_klass, to the ClassLoaderData's constructor where if 
> > > > _name and/or _name_and_id fields are null, a reasonable 
> > > > _class_loader_klass's external_name() can be provided.
> > > > 
> > > > open webrev http://cr.openjdk.java.net/~lfoltan/bug_jdk8205509/webrev/
> > > > bug link at https://bugs.openjdk.java.net/browse/JDK-8205509
> > > > 
> > > > Testing: hs-tier(1-2), jdk-tier(1-3) complete
> > > > hs-tier3-5 in progress
> > > > --test-repeat 50 ran for tier1 platforms of test 
> > > > jdk/java/lang/instrument/DaemonThread/TestDaemonThread.java
> > > > 
> > > > Thanks,
> > > > Lois
> > > 
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]