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

Karm Michal Babacek duke at openjdk.org
Thu Oct 24 08:13:18 UTC 2024


On Wed, 23 Oct 2024 19:54:17 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> 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 text.
> 
> 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.

Thank you @mrserb for your time. Let me amend the PR and run tests again.

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

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


More information about the client-libs-dev mailing list