RFR(XS): 8209736: runtime/RedefineTests/ModifyAnonymous.java fails with NullPointerException when running in CDS mode

[Calvin Cheung]

On 8/31/18, 1:52 PM, Jiangli Zhou wrote:
> Hi Calvin,
>
>
> On 8/31/18 1:33 PM, Calvin Cheung wrote:
>> Hi Jiangli,
>>
>> On 8/31/18, 1:10 PM, Jiangli Zhou wrote:
>>> Hi Calvin,
>>>
>>> How about use a separate loop (before the main test loop in 
>>> runTest()) to print out the class names in the allLoadedClasses array?
>>  109     static void runTest() {
>>  110         while (!done) {
>>  111             Class[] allLoadedClasses = inst.getAllLoadedClasses();
>>  112             int len = allLoadedClasses.length;
>>  113             for (int idx = 0; idx < len; idx++) {
>>
>> Did you mean print out all the class names after line 111? There 
>> would be too much output since there's a while loop enclosing it.
>> Or did you mean something else?
> Yes. If the failure does re-manifest again, we can compare the output 
> with a normal run. That would give some useful information. How big is 
> the allLoadedClasses?
On my local linux machine, it only loops through the while loop twice 
and I saw more than 1800 class names in the output.
>
> Or, maybe we could print the classes if null is detected.
This is a better option.
>>> If null is discovered, throw a RuntimeException(). That helps to 
>>> provide more debugging information and also will not silent the 
>>> issue that causes the NPE if it does' resurface again.
>> I don't think throwing a RuntimeException() gives anymore info than 
>> the existing NPE.
>> The idea of this change is that after a null clazz is encountered, 
>> let the test continue to run. It may fail in a different way which 
>> may give us more clues.
> The NPE might be caused by a rare race condition, which might not 
> affect the redefineClass process and the test could pass with the 
> change. We should avoid such case.
Maybe but we don't know for sure. The main purpose of the test is to 
make sure an anonymous class cannot be retransformed or redefined. So I 
don't think we're masking any problem if a null class is encountered and 
the test passed.

thanks,
Calvin
>
> Thanks,
> Jiangli
>>
>> thanks,
>> Calvin
>>>
>>> Thanks,
>>>
>>> Jiangli
>>>
>>>
>>> On 8/31/18 11:33 AM, Calvin Cheung wrote:
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8209736
>>>>
>>>> webrev: http://cr.openjdk.java.net/~ccheung/8209736/webrev.00/
>>>>
>>>> The test failure is no longer reproducible; it happened only once 
>>>> in tier3 and once in tier6 testing.
>>>> This simple change is just adding a null check on the variable 
>>>> clazz and prints the array size and index if it is null.
>>>>
>>>> Testing: hs-tier{1,2,3} with default CDS archive in JDK.
>>>>
>>>> thanks,
>>>> Calvin
>>>
>