On Thu, 3 Oct 2024 11:48:11 GMT, Karm Michal Babacek <[email protected]> wrote:

>> If there is a problem with finding and calling e.g. 
>> `java/awt/GraphicsEnvironment` in `AWTIsHeadless`, the env' Exception 
>> remains set and it is not cleared. Later, that manifests as:
>> 
>>     Fatal error reported via JNI: Could not allocate library name
>> 
>> Which is misleading. The code path is perhaps rare in a normal JDK usage, 
>> but it has been complicating our users' bug reports in the 
>> GraalVM/native-image ecosystem for quite some time.
>> 
>> Instead of failing later indicating that the user has incorrectly configured 
>> JNI, it bails out very soon with a message that seems as if a jstring could 
>> not have been allocated. It sends users on wild goose chases where it 
>> appears `JNU_NewStringPlatform` calls failed,  e.g.
>> 
>> * https://github.com/oracle/graal/issues/9138
>> * https://github.com/oracle/graal/issues/8475
>> * https://github.com/oracle/graal/issues/9300
>> * https://github.com/quarkusio/quarkus/issues/31596
>> * https://github.com/graalvm/mandrel/issues/292
>> * https://github.com/Karm/mandrel-integration-tests/issues/262
>> 
>> This commit fixes the error reporting in the AWTIsHeadless.
>> 
>> Furthermore, when AOT compiled, there is little sense for having a 
>> JAVA_HOME, yet some parts of AWT code look for it to search fonts. In such 
>> case, an empty directory structure is enough to accommodate it, e.g.
>> 
>> /tmp/JAVA_HOME/
>> /tmp/JAVA_HOME/conf
>> /tmp/JAVA_HOME/conf/fonts
>> /tmp/JAVA_HOME/lib
>> 
>> The exception is somewhat cryptic for users again, merely stating:
>> 
>>     Exception in thread "main" java.io.IOException: Problem reading font 
>> data.
>>         at [email protected]/java.awt.Font.createFont0(Font.java:1205)
>>         at [email protected]/java.awt.Font.createFont(Font.java:1076)
>>         at imageio.Main.loadFonts(Main.java:139
>> 
>> Adding the cause there makes it clearer, i.e. that JAVA_HOME might be 
>> missing:
>> 
>>     Exception in thread "main" java.io.IOException: Problem reading font 
>> data.
>>         at java.desktop@23-internal/java.awt.Font.createFont0(Font.java:1206)
>>         at java.desktop@23-internal/java.awt.Font.createFont(Font.java:1076)
>>         at imageio.Main.loadFonts(Main.java:139)
>>         at imageio.Main.paintRectangles(Main.java:97)
>>         at imageio.Main.main(Main.java:195)
>>         at 
>> java.base@23-internal/java.lang.invoke.LambdaForm$DMH/sa346b79c.invokeStaticInit(LambdaForm$DMH)
>>     Caused by: java.lang.Error: java.home property not set
>>         at 
>> java.desktop@23-internal/sun.awt.FontConfiguration.findFontConfigF...
>
> 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 a java exceptions as a 
fatal errors and fail fast
 - https://bugs.openjdk.org/browse/JDK-8130507: ignores  a 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.

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

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

Reply via email to