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 via email to