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

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

thanks,

Chris

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

David Holmes
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.

Thanks,
David

> thanks,
>
> Chris
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: 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@oracle.com
In reply to this post by Chris Plummer
Hi Chris,

Looks good.

Thanks,
Serguei


On 3/6/17 16:57, 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
>
> thanks,
>
> Chris
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

David Holmes
In reply to this post by David Holmes
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.

David

> Thanks,
> David
>
>> thanks,
>>
>> Chris
>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: 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 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.

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

David Holmes
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.

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
>>>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

David Holmes
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.

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
>>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: 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@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
>>>>>>>
>>>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: 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/7/17 1:38 AM, [hidden email] wrote:

> 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
David and Serguie, thanks for the reviews!

Chris

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

Loading...