RFR(S): JDK-8129108: nmethod related crash in CMS

[Mikael Gerdin]

Hi Poonam,

On 2015-06-22 23:19, Poonam Bajaj Parhar wrote:
> Hello Mikael,
>
> Thanks for taking a look at the changes.
>
> Yes, you are right, the best place to add the SO_AllCodeCache
> root_scanning option is
> CMSCollector::setup_cms_unloading_and_verification_state(). There the
> issue is that
> we are adding/removing the root scanning options based on if the
> verification options
> are enabled/disabled.
>
> SO_AllCodeCache should get added to the root_scanning_options if we are
> not unloading
> classes regardless whether we are verifying or not.
>
> I compared this code with the code of jdk7 and the only part that has
> changed in there is related
> to the Perm Gen. I could also reproduce the issue with 7u fastdebug
> build with
> -XX:+CMSParallelInitialMarkEnabled and -XX:-VerifyBeforeExit.
>
> Here's the updated webrev:
> http://cr.openjdk.java.net/~poonam/8129108/webrev.01/
>
> Failing testcase works fine with these changes.

I think the change seems ok, but I'd feel a lot safer if you had some 
more testing results.

/Mikael

>
> Thanks,
> Poonam
>
> On 6/22/2015 6:41 AM, Mikael Gerdin wrote:
>> Poonam,
>>
>> On 2015-06-20 06:32, Poonam Bajaj Parhar wrote:
>>> Hello,
>>>
>>> Please review this change for bug:
>>> JDK-8129108: <https://bugs.openjdk.java.net/browse/JDK-8129108> nmethod
>>> related crash in CM
>>>
>>> There is an assertion failure with fastdebug build with test
>>> /nsk.coverage.arguments.arguments002.arguments002/.
>>> This failure occurs when CMSCollector::_should_unload_classes is false
>>> and the _roots_scanning_options does
>>> not contain SO_AllCodeCache:
>>>
>>> #  Internal Error
>>> (hotspot/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp:4472),
>>> pid=16267, tid=0xf36ffb70
>>> #  assert(_collector->should_unload_classes() ||
>>> (_collector->CMSCollector::roots_scanning_options() &
>>> GenCollectedHeap::SO_AllCodeCache)) failed: if we didn't scan the code
>>> cache, we have to be ready to drop nmethods with expired weak oops
>>>
>>>
>>> This problem was exposed with the fix ofJDK-8085965:
>>> <https://bugs.openjdk.java.net/browse/JDK-8085965> VM hangs in
>>> C2Compiler,
>>> that disables CMSClassUnloadingEnabled when -Xnoclassgc or
>>> -XX:-ClassUnloading
>>> are specified on the command line.
>>>
>>> CMSCollector::_roots_scanning_options option should contain
>>> SO_AllCodeCache to ensure
>>> that we scan code cache when we are not unloading classes.
>>>
>>> Webrev: http://cr.openjdk.java.net/~poonam/8129108/webrev/
>>
>> This looks very similar to the code in
>> CMSCollector::setup_cms_unloading_and_verification_state
>> which is called from CMSCollector::checkpointRootsInitialWork
>>
>> I think that the if-clause in setup_cms_unloading... needs to be fixed
>> to handle the class unloading case separately from the verification
>> state.
>> It's not entirely clear to me exactly which cases the complicated
>> conditions in the if statements try to capture, though. You may want
>> to look at a previous version of the sources (before perm gen removal)
>> to better understand the reasoning behind it.
>>
>> /Mikael
>>
>>> Testing: JPRT, testcase
>>> /nsk.coverage.arguments.arguments002.arguments002/
>>>
>>> Thanks,
>>> Poonam
>>>
>