Bugfix on CMSClassUnloadingEnabled used with CMSClassUnloadingMaxInterval > 0

[Mikael Gerdin]

Hi Jungwoo,

On 2013-09-17 01:14, Jungwoo Ha wrote:
> Hi,
>
> We found that hotspot crashes when CMSClassUnloadingEnabled is true, and
> CMSClassUnloadingMaxInterval > 0.
> This is on hotspot24 and u40.
> This is easily reproducible with DaCapo tradesoap benchmark with
> heapsize around 200MB.
>
> The reason for the crash is that CMS sets the root set when
> CMSClassUnloadingEnabled is on during the constructor phase assuming
> that every CMS cycle will unload the class.
> However, when CMSClassUnloadingMaxInterval > 0, CMS may not unload
> classes ended up crashing.
> I think this is apparently a bug, and I attach the fix.
> Please take a look at the attached patch.
> My changes are resetting the root scanning option on every CMS cycle in
> setup_cms_unloading_and_verification_state() if CMSCLassUnloadingEnabled
> is on.
>
> Please take a look and let us know how to proceed.

Jon filed a bug for this:
https://bugs.openjdk.java.net/browse/JDK-8024954

I think the code change fixes the bug but I'd like it better if you 
changed the code to always set the requested ScanOption values in 
setup_cms_unloading_and_verification_state

something like:
if (should_unload_classes()) {
set_root_scanning_option(SO_SystemClasses);
} else {
set_root_scanning_option(SO_AllClasses|SO_Strings|SO_CodeCache);
}

I also believe that your check for CMSClassUnloadingEnabled can cause a 
bug similar to the one you are trying to fix if 
ExplicitGCInvokesConcurrentAndUnloadsClasses would be enabled.
should_unload_classes() should be sufficient to determine which 
ScanOptions to set for a particular cms cycle.

Please send a patch/webrev based on the latest jdk8 sources 
http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot
Since the function is also messing around with the verification flags, 
make sure to run some tests on your changes with verification enabled.

/Mikael

>
> Thanks,
> Jungwoo