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
