RFR (L) 8213501 : Deploy ExceptionJniWrapper for a few tests

[JC Beyler]

Hi all,

Anybody motivated to review this? :)
Jc

On Wed, Nov 7, 2018 at 9:53 PM JC Beyler <jcbeyler at google.com> wrote:

> Hi all,
>
> Could I have a review for the extension and usage of the
> ExceptionJniWrapper. This adds lines and filenames to the end of the
> wrapper JNI methods, adds tracing, and throws an error if need be. I've
> ported the gc/lock files to use the new TRACE_JNI_CALL add-on and I've
> ported a few of the tests that were already changed for the assignment
> webrev for JDK-8212884.
>
> Webrev: http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.00/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
>
> For illustration, if I force an error to the AP04/ap04t03 test and set the
> verbosity on, I get something like:
>
> >> Calling JNI method FindClass from ap04t003.cpp:343
> >> Calling with these parameter(s):
>         java/lang/Threadd
> Wait for thread to finish
> << Called JNI method FindClass from ap04t003.cpp:343
> Exception in thread "Thread-0" java.lang.NoClassDefFoundError:
> java/lang/Threadd
>         at
> nsk.jvmti.scenarios.allocation.AP04.ap04t003.runIterateOverHeap(Native
> Method)
>         at
> nsk.jvmti.scenarios.allocation.AP04.ap04t003HeapIterator.runIteration(ap04t003.java:140)
>         at
> nsk.jvmti.scenarios.allocation.AP04.ap04t003Thread.run(ap04t003.java:201)
> Caused by: java.lang.ClassNotFoundException: java.lang.Threadd
>         at
> java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:583)
>         at
> java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
>         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
>         ... 3 more
> FATAL ERROR in native method: JNI method FindClass : internal error from
> ap04t003.cpp:343
>         at
> nsk.jvmti.scenarios.allocation.AP04.ap04t003.runIterateOverHeap(Native
> Method)
>         at
> nsk.jvmti.scenarios.allocation.AP04.ap04t003HeapIterator.runIteration(ap04t003.java:140)
>         at
> nsk.jvmti.scenarios.allocation.AP04.ap04t003Thread.run(ap04t003.java:201)
>
> Questions/comments I have about this are:
>   - Do we want to force fatal errors when a JNI call fails in general?
> Most of these tests do the right thing and test the return of the JNI
> calls, for example:
>     thrClass = jni->FindClass("java/lang/Threadd", TRACE_JNI_CALL);
>     if (thrClass == NULL) {
>
> but now the wrapper actually would do a fatal if the FindClass call would
> return a nullptr, so we could remove that test altogether. What do you
> think?
>     - I prefer to leave them as the tests then become closer to what real
> users would have in their code and is the "recommended" way of doing it
>
>    - The alternative is to use the NonFatalError I added which then just
> prints out that something went wrong, letting the test continue. Question
> will be what should be the default? The fatal or the non-fatal error
> handling?
>
> On a different subject:
>   - On the new tests, I've removed the NSK_JNI_VERIFY since the JNI
> wrapper handles the tracing and the verify in almost the same way; only
> difference I can really tell is that the complain method from NSK has a max
> complain before stopping to "complain"; I have not added that part of the
> code in this webrev
>
> Once we decide on these, I can continue on the files from JDK-8212884 and
> then do both the assignment in an if extraction followed-by this type of
> webrev in an easier fashion. Depending on decisions here, NSK*VERIFY can be
> deprecated as well as we go forward.
>
> Thank you for the reviews/comments :)
> Jc
>


-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181116/072570b8/attachment-0001.html>