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

[Chris Plummer]

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);

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
>>>>>
>>