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 <jcbey...@google.com
<mailto:jcbey...@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
