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

[David Holmes]

This is already primarily a runtime change with some GC test changes and 
serviceability test changes AFAICS!

David

On 18/11/2018 5:16 pm, Chris Plummer wrote:
> Hi JC,
> 
> I'll try to take a look early this week. Won't this eventually affect 
> non-svc tests? If so, I think the general mechanism should be reviewed 
> by a wider audience.
> 
> cheers,
> 
> Chris
> 
> On 11/16/18 7:43 PM, JC Beyler wrote:
>> Hi all,
>>
>> Anybody motivated to review this? :)
>> Jc
>>
>> On Wed, Nov 7, 2018 at 9:53 PM JC Beyler <jcbeyler at google.com 
>> <mailto: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/
>>     <http://cr.openjdk.java.net/%7Ejcbeyler/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
> 
>