Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JNIRegistrationAwt::registerFreeType called for sun.font.FontConfigManager too #3417

Closed
wants to merge 1 commit into from

Conversation

Karm
Copy link
Contributor

@Karm Karm commented May 17, 2021

Fixes #3416

With this patch, registerFreeType gets properly called and the build works. The resulting binary runs as expected and lists available fonts. Steps to reproduce are described on #3416. Copy-pasting here for clarity:

The following code, Main.java:

import java.awt.Font;
import java.awt.GraphicsEnvironment;

public class Main {
    public static void main(String[] args) {
        for (Font font : GraphicsEnvironment.getLocalGraphicsEnvironment().getAllFonts()) {
            System.out.printf("%s, %s\n", font.getFontName(), font.getFamily());
        }
    }
}

compiled as:

export JAVA_HOME=/home/karm/tmp/graal-labsjava11-build;export GRAALVM_HOME=${JAVA_HOME};export PATH=${JAVA_HOME}/bin:${PATH}
javac Main.java
java -agentlib:native-image-agent=config-output-dir=./META-INF/native-image Main
native-image --no-fallback --initialize-at-build-time= -H:DeadlockWatchdogInterval=2 --native-image-info --verbose Main

The occasional deadlock mentioned in the original issue #3416 still occurs, although I am not sure it's not my system's fault so I ll be debugging and pursuing that in a separate issue/PR.

Feedback is welcome, I am definitely not sure I am approaching this the right way.

@Karm
Copy link
Contributor Author

Karm commented May 18, 2021

See old #2729

@Karm
Copy link
Contributor Author

Karm commented May 18, 2021

Ad CI, I don't get what's wrong from the log in E:133,15: Access to member 'results' before its definition line 134 (access-member-before-definition)

Ideas?

Running pylint on /home/runner/work/graal/graal/sulong/mx.sulong/mx_sulong_suite_constituents.py...
************* Module mx_sulong_suite_constituents
I: 49, 0: Locally disabling E0602 (locally-disabled)
I:191, 0: Locally disabling R0901 (locally-disabled)
E:133,15: Access to member 'results' before its definition line 134 (access-member-before-definition)

Traceback (most recent call last):
  File "/home/runner/work/graal/graal/mx/mx_gate.py", line 422, in gate
gate: 17 May 2021 18:23:37(+00:21) END:   Pylint [0:00:16.470541] [disk (free/total): 18.7GB/83.2GB]
    _run_gate(cleanArgs, args, tasks)
  File "/home/runner/work/graal/graal/mx/mx_gate.py", line 569, in _run_gate
    if mx.command_function('pylint')(['--primary']) != 0:
  File "/home/runner/work/graal/graal/mx/mx_commands.py", line 147, in __call__
    return self.command_function(*args, **kwargs)
  File "/home/runner/work/graal/graal/mx/mx.py", line 14464, in pylint
    run([pylint_exe, '--reports=n', '--rcfile=' + rcfile, pyfile] + additional_options, env=env)
  File "/home/runner/work/graal/graal/mx/mx.py", line 13030, in run
gate: 17 May 2021 18:23:37(+00:21) ABORT: Gate [0:00:21.885427] [disk (free/total): 18.7GB/83.2GB]
    abort(retcode)
  File "/home/runner/work/graal/graal/mx/mx.py", line 4082, in abort
    raise SystemExit(error_code)
SystemExit: 2

The sequence of mx commands that were executed until the failure follows:

mx --primary-suite-path sulong --J @-Xmx2g --java-home=/home/runner/work/graal/graal/jdk version --oneline
mx --primary-suite-path sulong --J @-Xmx2g --java-home=/home/runner/work/graal/graal/jdk sversions
mx --primary-suite-path sulong --J @-Xmx2g --java-home=/home/runner/work/graal/graal/jdk verifymultireleaseprojects
mx --primary-suite-path sulong --J @-Xmx2g --java-home=/home/runner/work/graal/graal/jdk pylint --primary

If the previous sequence is incomplete or some commands were executed programmatically use:

mx --primary-suite-path sulong --J @-Xmx2g --java-home=/home/runner/work/graal/graal/jdk gate --strict-mode --tags style,fullbuild,sulongBasic

2
Error: Process completed with exit code 1.

@zakkak
Copy link
Collaborator

zakkak commented May 19, 2021

Ad CI, I don't get what's wrong from the log in E:133,15: Access to member 'results' before its definition line 134 (access-member-before-definition)

Ideas?

That's irrelevant to this PR, it appears that this test has been failing for some time now.

@Karm
Copy link
Contributor Author

Karm commented May 20, 2021

Hello @christianwimmer, is this PR O.K. or am I merely covering up a deeper problem?

@christianwimmer
Copy link

I don't think anything related to AWT can be reasonably expected to work with --initialize-at-build-time=, i.e., with initializing all classes at image build time. There is so much static state in AWT classes that it is more or less impossible to check which fields would need a reset / recomputation manually.

Since sun.font.FontConfigManager itself does not do any System.loadLibrary, it seems like a very random class to use as an additional trigger. You would need to actually list all classes whose class initializer contains a call to FontManagerNativeLibrary.load(). Because when AWT classes are initialized at image build time, then the static analysis does not see the call chains to FontManagerNativeLibrary that currently trigger the registration. But that does not solve all the other static-state problems of AWT.

@pejovica what do you think?

@pejovica
Copy link
Member

Unfortunately, this is just covering up a deeper problem. To illustrate this, consider the class initializer of FreetypeFontScaler:

    static {
        /* At the moment fontmanager library depends on freetype library
           and therefore no need to load it explicitly here */
        FontManagerNativeLibrary.load();
        initIDs(FreetypeFontScaler.class);
    }

By initializing FreetypeFontScaler at image build time, its <clinit> won't be considered by static analysis, but more importantly it won't be executed at runtime, which is wrong. In particular, initIDs is a native method that initializes a native state, so without calling it, we would have an uninitialized native state at runtime, which is a segfault in disguise. On the other hand, FontManagerNativeLibrary.load is a hook to trigger loading of awt and fontmanager libraries using System.loadLibrary in <clinit> of FontManagerNativeLibrary. However, since FontManagerNativeLibrary was also initialized at image build time, analysis won't see these System.loadLibrary calls (hence the missing symbols) and they will also not be executed at runtime. This means that from a VM perspective these libraries will not be loaded during execution, and even their JNI_OnLoad functions will not be called, which is again wrong.

The illustrated problems are obviously just the tip of the iceberg. The bottom line is that initializing all classes at image build time without prior close inspection of all relevant code (both Java and native) will result in fundamentally broken images. In other words, the fact that a given example works is nothing more but a mere coincidence.

That said, we are not against making --initialize-at-build-time= work with AWT, but what worries me is that the amount of workarounds needed would not be maintainable in the long run.

@christianwimmer
Copy link

Summary: Initializing all classes at image build time for AWT/Swing is not feasible at this time. It would require quite extensive patching of the JDK code, which cannot be done in a maintainable way using substitutions. The only proper way would be to make changes to the JDK itself, i.e., contribute to OpenJDK.
Since there is no need to use --initialize-at-build-time=, removing that from the native-image command line is the recommended solution for now.

@Karm
Copy link
Contributor Author

Karm commented May 25, 2021

Thank you @christianwimmer for the summary.

Thanks @pejovica for taking the time to explain it in detail. I learned a lot 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants