<AWT Dev> RFR: 8257853: Remove dependencies on JNF's JNI utility functions in AWT and 2D code [v6]

Sergey Bylokhov serb at openjdk.java.net
Fri Dec 18 21:37:01 UTC 2020

On Fri, 18 Dec 2020 03:51:43 GMT, Phil Race <prr at openjdk.org> wrote:

>> src/java.desktop/macosx/native/libosxapp/JNIUtilities.h line 152:
>>> 150: #define CHECK_EXCEPTION() \
>>> 151:     if ((*env)->ExceptionOccurred(env) != NULL) { \
>>> 152:         (*env)->ExceptionClear(env); \
>> Why an exception is cleared here? Is it really match the behavior before the fix? I guess before the fix various JNF methods thrown the objective-c exceptions which are usually handled by the JNF_COCOA_ENTER/EXIT or in the NSApplicationAWT.runAWTLoopWithApp()
> The behaviour may not be identical but it is non-identical in theoretical fatal cases.
> We are actually better off here. I am seeing fewer JNI warning as a result.
> Also NS exceptions per Apple should result in termination of the process.
> I don't think JNF ever did the right thing.
> So as I am sure you know this is only ever going to come into play if there is some JNI
> error which indicates an internal error. I don't think we are any worse off here.

I see a mixing approach here, on other platforms like on windows we:
 - Use such macro in the initid() method in the static initializers and any errors are fatal in that case. And we usually use the macro instead of functions, because it is possible to check and return from the initid().
 - If some exception like OOM has occurred on the toolkit thread we wrap it to the std::bad_alloc end stop the application.

The JNF solution on macOS merges two solutions above, so we call JNI methods in any place and the library transforms all possible java exceptions to the NSExceptions, and back. And it was quite useful, much better than the list of macro.

>> src/java.desktop/macosx/native/libosxapp/JNIUtilities.h line 120:
>>> 118:      } \
>>> 119:      LOG_NULL(dst_var, name); \
>>> 120:      CHECK_NULL(dst_var);
>> If some of these macros will be used on appkit thread, then the check like "CHECK_NULL" will not work,  because the code will never return to the java, and the failure will be actually ignored.
> If we get NULL back then there's a progamming bug. LOG_NULL will show it.
> CHECK_NULL is just as much to satisfy JNI as anything.
> If we are on a Java thread then it will still add some value so I don't see the point in removing it.
> Nor do I think I'd like to use NSException instead because that affects the flow of control.
> Yes, this may be different than before (probably more relevant to the other comment) but I think better may be a better approach overall. If we ever see an issue here that is likely to occur due to anything other than a bad internal inconsistency due to JDK changes then I would prefer to follow the code path and fix it up.

I mean all macro, not just CHECK_NULL. You just create a JNF library 1.5 which for some reason uses macro instead of objective-c functions, uses logging instead of exceptions, and changes a huge number of files. Does it make the codebase better?
 - I think we should ask Apple to contribute the JNF library or merge it as a third-party library, then cut all unused parts of it
 - Or we should create JNF library 2.0, and the whole fix will be looking like a replacement of three letters in each function, like JNFCallVoidMethod - > JNUCallVoidMethod etc.


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

More information about the awt-dev mailing list