RFR(10)(XXS): 8175341: "java/util/Arrays/ParallelPrefix.java" Crash Internal Error ...diagnosticCommand.cpp...assert(k != __null) failed: FinalizerHistogram class is not accessible

[serguei.spitsyn at oracle.com]

On 3/6/17 23:15, David Holmes wrote:
> Sorry for the delay TB decided not to pull any email down from my 
> openjdk folder for 3 hours :(
>
> On 7/03/2017 3:30 PM, Chris Plummer wrote:
>> On 3/6/17 6:25 PM, David Holmes wrote:
>>> On 7/03/2017 12:14 PM, Chris Plummer wrote:
>>>> On 3/6/17 5:58 PM, David Holmes wrote:
>>>>> On 7/03/2017 11:51 AM, David Holmes wrote:
>>>>>> Hi Chris,
>>>>>>
>>>>>> On 7/03/2017 10:57 AM, Chris Plummer wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Please review the following simple fix. Explanation of the cause 
>>>>>>> and
>>>>>>> the
>>>>>>> fix are in the CR. Testing so far has only been done with
>>>>>>> serviceability/dcmd/gc/FinalizerInfoTest.java as described in the
>>>>>>> CR. I
>>>>>>> think the addition of JPRT push testing will be sufficient.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~cjplummer/8175341/webrev.00/webrev.hotspot/ 
>>>>>>>
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8175341
>>>>>>
>>>>>> Looking at the other potentially-exception-throwing actions in that
>>>>>> DCmd
>>>>>> and others, I think a simple delete of the assert and change:
>>>>>>
>>>>>>  425     vmSymbols::finalizer_histogram_klass(), THREAD);
>>>>>>
>>>>>> to
>>>>>>
>>>>>>  425     vmSymbols::finalizer_histogram_klass(), CHECK);
>>>>>>
>>>>>> would suffice. Something upstream has to handle potential errors
>>>>>> and the
>>>>>> pending exception in any case.
>>>>>
>>>>> Oops! This explains why no exception:
>>>>>
>>>>> 424   Klass* k = SystemDictionary::resolve_or_null(
>>>>>  425     vmSymbols::finalizer_histogram_klass(), THREAD);
>>>>>
>>>>> it uses resolve_or_null, not resolve_or_fail! I think it should use
>>>>> the latter and simply throw like the other Dcmds.
>>>>>
>>>> Ok. That seems to work. Bit of a hurry here before I step out, so
>>>> trimmed diff is inline:
>>>>
>>>> -  Klass* k = SystemDictionary::resolve_or_null(
>>>> -    vmSymbols::finalizer_histogram_klass(), THREAD);
>>>> -  assert(k != NULL, "FinalizerHistogram class is not accessible");
>>>> +  Klass* k = SystemDictionary::resolve_or_fail(
>>>> +    vmSymbols::finalizer_histogram_klass(), true, CHECK);
>>>> +  if (k == NULL) {
>>>> +    return;
>>>> +  }
>>>>
>>>> I'm a bit unclear about THREAD vs CHECK. They both seem to work. Not
>>>> sure which is correct here.
>>>
>>> THREAD is just a reference to the current thread (as passed down the
>>> call chain). CHECK automatically expands into a pending exception
>>> check plus a return. So your "if (k==null) ..." is unreachable with
>>> CHECK - as k will only be NULL if an exception is pending.
>> Ok. This seems to work as well:
>>
>> -  Klass* k = SystemDictionary::resolve_or_null(
>> -    vmSymbols::finalizer_histogram_klass(), THREAD);
>> -  assert(k != NULL, "FinalizerHistogram class is not accessible");
>> +  Klass* k = SystemDictionary::resolve_or_fail(
>> +    vmSymbols::finalizer_histogram_klass(), true, CHECK);
>
> Yep looks good to me.

+1

Thanks,
Serguei


>
> Thanks,
> David
> -----
>
>> thanks,
>>
>> Chris
>>>
>>> Thanks,
>>> David
>>>
>>>> When I forced an exception by dropping the last character of the class
>>>> name, here's what I see in the test output:
>>>>
>>>>  stderr: [java.lang.NoClassDefFoundError:
>>>> java/lang/ref/FinalizerHistogra
>>>> ]
>>>>
>>>> cheers,
>>>>
>>>> Chris
>>>>> David
>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>> thanks,
>>>>>>>
>>>>>>> Chris
>>>>>>>
>>>>
>>