Bugfix on CMSClassUnloadingEnabled used with CMSClassUnloadingMaxInterval > 0

[Jungwoo Ha]

Mikael,

I updated the patch as you mentioned.
There are no "set"_root_scanning_option, so I added that.
http://cr.openjdk.java.net/~rasbold/8024954/webrev/

Jungwoo


On Thu, Sep 19, 2013 at 4:48 AM, Mikael Gerdin <mikael.gerdin at oracle.com>wrote:

> 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<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
> ExplicitGCInvokesConcurrentAnd**UnloadsClasses 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<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
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130924/58576113/attachment.htm>