RFR: 8259343: [macOS] Update JNI error handling in Cocoa code.

[Sergey Bylokhov]

On Fri, 8 Jan 2021 02:40:58 GMT, Phil Race <prr at openjdk.org> wrote:

>> src/java.desktop/macosx/native/libosxapp/JNIUtilities.h line 41:
>> 
>>> 39:        NSLog(@"%@",[NSThread callStackSymbols]); \
>>> 40:        if ([NSThread isMainThread] == NO) { \
>>> 41:            JNU_ThrowInternalError(env, "Bad JNI Lookup"); \
>> 
>> It will be possible that(most of the time) the JNU_ThrowInternalError will be called while we already have a raised java exception.
>
> I don't think "most" of the time is very likely. JNI lookup failures don't cause exceptions to be thrown.
> Is that what you are tihinking ?
> 
> And separately, with either the current code of clearing exceptions or the change here to  the practice of throwing NSException on seeing a Java Exception I think it very unlikely.

Then how we get NoSuchMethodError here? https://bugs.openjdk.java.net/browse/JDK-8259232?
https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#GetMethodID
Calling Instance Methods: GetMethodID
RETURNS:
    Returns a method ID, or NULL if the specified method cannot be found.
THROWS:
    NoSuchMethodError: if the specified method cannot be found.
    ExceptionInInitializerError: if the class initializer fails due to an exception.
    OutOfMemoryError: if the system runs out of memory.

Same for fields:
https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#GetFieldID

>> src/java.desktop/macosx/native/libosxapp/JNIUtilities.h line 180:
>> 
>>> 178:  * nature of the problem that has been detected and how survivable it is.
>>> 179:  */
>>> 180: #define CHECK_EXCEPTION() \
>> 
>> Since this macro does not try to return from the outer code we can change it to the method?
>
> But then "env" would need to be passed explicitly. I don't think the churn is worth it.
> And a method would then need a .m or .c file ...

But that could be merged to the CallXXXMethod and placed somewhere in the libosxapp

>> src/java.desktop/macosx/native/libosxapp/JNIUtilities.h line 182:
>> 
>>> 180: #define CHECK_EXCEPTION() \
>>> 181:     if ((*env)->ExceptionOccurred(env) != NULL) { \
>>> 182:         if ([NSThread isMainThread] == YES) { \
>> 
>> Isn't it is a similar code to the
>> https://github.com/openjdk/jdk/blob/67c221148159d94142a3a6d9ddadce2dff8c858b/src/java.desktop/macosx/native/libosxapp/NSApplicationAWT.m#L334
>> Where we just catch an exception log it, and continue to run our infinite loop? Why do we need to do something specific here? I mean we cannot drop any try/catch blocks anyway since an nsexception may be generated by some external system code.
>
> In my testing that code does not get called.  The new code does. So the old code could probably be deleted as useless but it is also harmless and just maybe it'll catch something else, even if it isn't doing anything that I can see.

That code for sure should be called, it is even improved recently by JDK-8255681
I'll check how it was intended to work.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1967