RFR: 8336382: Fixes error reporting in loading AWT and fonts [v9]

[Sergey Bylokhov]

On Fri, 18 Oct 2024 19:49:07 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> Karm Michal Babacek has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Amends test description
>
> src/java.desktop/unix/native/libawt/awt/awt_LoadLibrary.c line 70:
> 
>> 68:         graphicsEnvClass = (*env)->FindClass(env,
>> 69:                                              "java/awt/GraphicsEnvironment");
>> 70:         CHECK_EXCEPTION_FATAL(env, "java/awt/GraphicsEnvironment class not found");
> 
> Taking out any discussion about graal/etc as well as absent of any classes and methods, I agree that the current code is questionable.
> 
> But the solution to always print "java/awt/GraphicsEnvironment class not found" does not seems to be correct as well.
>  
> One of the problem with this code is an actual conflict between two patches. Both are related to warning caused by pending java exceptions, but implemented differently:
>  - https://bugs.openjdk.org/browse/JDK-8031001: treated the java exceptions as a fatal errors and fail fast
>  - https://bugs.openjdk.org/browse/JDK-8130507: ignores  the java exceptions and tried to fallback into the headless mode in assumption that it may be fine for the application
> 
> Both of that patches only changes the code paths which were reported by "some tools"/checkjni - this is the reason why the next code was not reported and was not patched(which is wrong):
> 
> 
>        graphicsEnvClass = (*env)->FindClass(env,
>                                              "java/awt/GraphicsEnvironment");
>         if (graphicsEnvClass == NULL) {
>             return JNI_TRUE;
>         }
>         headlessFn = (*env)->GetStaticMethodID(env,
>                                                graphicsEnvClass, "isHeadless", "()Z");
>         if (headlessFn == NULL) {
>             return JNI_TRUE;
>         }
> 
> If we would like to follow JDK-8031001 approach we should use fatal error here and fail fast, for example if OOM is occurred.
> If we would like to follow JDK-8130507 we should clear an exceptions and try to use headless mode.
> 
> I guess ignoring an exceptions is not that good and the fix for JDK-8130507 should be rethinking.

You can change the CHECK_EXCEPTION_FATAL macro to something like

#define CHECK_EXCEPTION_FATAL(env, message) \
    if ((*env)->ExceptionCheck(env)) { \
        (*env)->ExceptionDescribe(env);
        (*env)->FatalError(env, message); \
    }

In this case the root cause of the bug will always be printed.
And then update the fatal message to some generic test.

Note that the FatalError is used in this code since we always should load the library(libawt_xawt or libawt_headless) or fail fast, otherwise we most probably will get an error later.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20169#discussion_r1813446904