Hi Jc,
On 18/09/2018 1:59 PM, JC Beyler wrote:
Hi all,
I've slowly started working on JDK-8191519
<https://bugs.openjdk.java.net/browse/JDK-8191519>. However before
starting to really refactor all the tests, I thought I'd get a few
opinions. I am working on internalizing the error handling of JNI calls
via a SafeJNIEnv class that redefines all the JNI calls to add an error
checker.
The advantage is that the test code will look and feel like normal JNI
code and calls but will have the checks we want to have for our tests.
Not sure I get that. Normal JNI code has to check for errors/exceptions
after every call. The tests need those checks too. Today they are
explicit, with this change they become implicit. Not sure what we are
gaining here ??
If agreed with this, we can augment the SafeJNIEnv class as needed.
Also, if the tests require something else than fatal errors, we can add
a different marker and make it a parameter to the base class.
Webrev: http://cr.openjdk.java.net/~jcbeyler/8210842/webrev.00/
<http://cr.openjdk.java.net/%7Ejcbeyler/8210842/webrev.00/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8210842
Let me know what you think,
Two initial suggestions:
1. FatalOnException should be constructed with a description string so
that it can report the failing operation when calling FatalError rather
than the general "Problem with a JNI call".
2. Make the local SafeJNIEnv a pointer called env so that the change is
less disruptive. All the env->op() calls will remain and only the local
error checking will be removed.
The switch from, e.g., checking NULL returns to checking for pending
exceptions, need to be sure that the JNI operations clearly specify that
NULL implies there will be an exception pending. This change may be an
issue for static code analysis if not smart enough to understand the
RAII wrappers.
Thanks,
David
Jc
