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

[Vladimir Kozlov]

Looks fine to me.

Thanks,
Vladimir

On 8/13/14 2:18 AM, Tobias Hartmann wrote:
> 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
>>
>