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

Jiangli Zhou jiangli.zhou at oracle.com
Tue Sep 4 19:05:57 UTC 2018

Hi Calvin,

Thanks for making the updates. It would be useful to add a comment 
explaining the extra log is intended for assisting debugging when null 
is detected in the allLoadedClasses array in rare cases. No new webrev 
is needed for me.



On 9/4/18 11:53 AM, Calvin Cheung wrote:
> Hi Jiangli,
> Here's an updated webrev with your suggestions:
> - logging the loaded class names into a file;
> - throws a RuntimeException upon encountering a null clazz.
> webrev: http://cr.openjdk.java.net/~ccheung/8209736/webrev.01/
> thanks,
> Calvin
> On 8/31/18, 3:47 PM, Jiangli Zhou wrote:
>>> On Aug 31, 2018, at 2:54 PM, Calvin 
>>> Cheung<calvin.cheung at oracle.com>  wrote:
>>>> 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.
>> Probably dumping to a file.
>>>>>> 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.
>> The issue is with getAllLoadedClasses. The proposed change would make 
>> the issue not being detected. I think we need to understand what’s 
>> the expected behavior of the API.
>> Thanks,
>> Jiangli
>>> 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

More information about the hotspot-runtime-dev mailing list