RFR 8165246: [REDO] InstanceKlass::_previous_version_count goes negative

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Wed Sep 7 18:52:10 UTC 2016


Hi Coleen,

I saw the review comments from George and Dan and so, do not touch this 
ground.
The fix looks good in general but I have a couple of comments.

The _has_previous_versions being a static field looks a bit dangerous to me,
but it is probably Ok as the function has_previous_versions() is called only
in one place in the do_unloading which then will recalculate the filed 
value.


src/share/vm/oops/instanceKlass.cpp

3389 void InstanceKlass::purge_previous_version_list() {
3390 assert(SafepointSynchronize::is_at_safepoint(), "only called at 
safepoint");
3391 assert (has_been_redefined(), "Should only be called for main class");
3392
3393 if (previous_versions() != NULL) { . . . 3476   }

   Nit: I'd suggest to replace the 3393+3476 with:

if (previous_versions() == NULL) { return; }

   It will simplify the code by reducing the nesting level and 
indentation. Thanks, Serguei On 9/6/16 09:24, Coleen Phillimore wrote:
> Summary: make _has_previous_version a boolean that is set to true when 
> previous version of a class is added or during class unloading call to 
> purge_previous_versions The failed fix was because InstanceKlass's 
> previous_versions klasses were walked more than once during class 
> unloading, because the scratch classes are also on the CLD::_klasses 
> list.  This change does three things.  1. Only walk the class if 
> has_been_redefined (which is not set for the scratch class), 2. set 
> next link to NULL for the scratch class when removed from the previous 
> version list, and 3. change the counter to a flag because the counter 
> may never go to zero if a CLD has redefined classes with previous 
> versions and is unloaded.  For this latter case we don't need to walk 
> previous_versions. I tested this with changes 1 and 2 in Kitchensink, 
> to verify the diagnosis, and then added 3 and retested. Also tested 
> with the fix for https://bugs.openjdk.java.net/browse/JDK-8156137 
> which was checked into hs_-omp repository. Also ran through RBT 4 
> tiers nightly tests. open webrev at 
> http://cr.openjdk.java.net/~coleenp/8165246.01/webrev bug link 
> https://bugs.openjdk.java.net/browse/JDK-8165246 Thanks, Coleen 



More information about the hotspot-runtime-dev mailing list