[9] RFR(S): 8054402: "klass->is_loader_alive(_is_alive)) failed: must be alive" for anonymous classes

[Tobias Hartmann]

Vladimir, Coleen, thanks for the review.

Please find comments inline.

On 12.08.2014 16:11, Coleen Phillimore wrote:
> There is some dead code (if statement) in dictionary::do_unloading() 
> when you remove this flag that you can remove also.

Thanks for pointing that out. As you suggested, I also moved the index 
into the loop statement.

> Also, about the test.  I thought AnonymousClass was something we were 
> going to discontinue support for (or was it a different AnonymousClass)?

I'm now directly using the corresponding method of the Unsafe API 
instead of the AnonymousClassLoader.

> On 8/12/14, 9:49 AM, Vladimir Kozlov wrote:
>> Should you also modify Dictionary::do_unloading() since it is the 
>> only place where it is called and the result is not used anymore?

Yes, I missed that. Done.

>> I don't think you need IgnoreUnrecognizedVMOptions flag in the test.

I thought we need it for the Whitebox API but that's not the case. I 
removed it.

New webrev: http://cr.openjdk.java.net/~thartmann/8054402/webrev.01/

Thanks,
Tobias

>>
>> Thanks,
>> Vladimir
>>
>> On 8/12/14 2:20 AM, Tobias Hartmann wrote:
>>> Hi,
>>>
>>> please review the following patch that fixes JDK-8054402.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8054402
>>> Webrev: http://cr.openjdk.java.net/~thartmann/8054402/webrev.00/
>>>
>>> == Problem ==
>>> The test [1] rarely fails with the assert 
>>> 'klass->is_loader_alive(_is_alive)) failed: must be alive' in
>>> 'CheckClass::check_class' (nmethod.cpp) because a IC in a compiled 
>>> method [2] references an unloaded anonymous Klass*
>>> [3] (i.e., the Klass has an unloaded mirror class [4]). Usually, ICs 
>>> are cleaned in 'nmethod::do_unloading' by
>>> 'clean_ic_if_metadata_is_dead' if they reference stale metadata. In 
>>> this case, the check for stale metadata is not
>>> performed because 'unloading_occurred' is set to false and therefore 
>>> no class unloading should have happened.
>>> The error is in 'SystemDictionary::do_unloading'. First, 
>>> 'ClassLoaderDataGraph::do_unloading ' is invoked to remove the
>>> class loader data of unloaded classes. Next, 
>>> 'Dictionary::do_unloading' is called to remove the system dictionary
>>> entries of dead classes. 'unloading_occurred' is only set to true if 
>>> dead class loaders were found _and_ entries were
>>> removed from the system dictionary. The problem is that anonymous 
>>> classes are not in the system dictionary. In the rare
>>> case where only anonymous classes were unloaded, 
>>> 'unloading_occurred' is set to false even if (anonymous) class
>>> unloading occurred.
>>>
>>> == Solution ==
>>> The variable 'unloading_occurred' is always set to true if dead 
>>> class loaders were found.
>>> I was able to write a jtreg test that deterministically triggers the 
>>> bug.
>>>
>>> == Testing ==
>>> - New jtreg test
>>> - Failing test [1]
>>> - JPRT
>>>
>>> Thanks,
>>> Tobias
>>>
>>>
>>> [1] 
>>> java/util/stream/test/org/openjdk/tests/java/util/stream/InfiniteStreamWithLimitOpTest.java
>>> [2] java.util.stream.Streams$RangeIntSpliterator::forEachRemaining
>>> [3] 
>>> java.util.stream.StreamSpliterators$IntWrappingSpliterator$$Lambda$170/25231275
>>> [4] 
>>> java/util/stream/StreamSpliterators$IntWrappingSpliterator$$Lambda$170
>