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.

Sign up for GitHub

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

Do not load java.awt.Component and java.beans.Customizer classes when they are not used in Java Beans #5065

Closed
wants to merge 4 commits into from
Closed

Do not load java.awt.Component and java.beans.Customizer classes when they are not used in Java Beans #5065

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
69e3491
Do not load java.awt.Component and java.beans.Customizer classes when…
AlexanderScherbatiy Sep 23, 2022
ee603e4
Change the ordinary comment to a doc comment
AlexanderScherbatiy Oct 4, 2022
a1a4c6e
Add reachability handler for java.awt.Component class
AlexanderScherbatiy Oct 12, 2022
deed713
Remove unused getComponentClass() method
AlexanderScherbatiy Oct 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions substratevm/mx.substratevm/suite.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@
"java.desktop": [
"sun.java2d",
"sun.java2d.pipe",
"com.sun.beans.finder",
],
"java.management": [
"com.sun.jmx.mbeanserver",
Expand Down
62 changes: 62 additions & 0 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JavaBeansSubstitutions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright (c) 2022, BELLSOFT. All rights reserved.
* Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package com.oracle.svm.core.jdk;

import com.oracle.svm.core.annotate.Substitute;
import com.oracle.svm.core.annotate.TargetClass;
import com.sun.beans.finder.ClassFinder;

@SuppressWarnings({"static-method", "unused"})
public final class JavaBeansSubstitutions {
// Checkstyle: stop

// Do not make string final to avoid class name interception
// in Class.forName(...) call
private static String COMPONENT_CLASS = "java.awt.Component";
private static String CUSTOMIZER_CLASS = "java.beans.Customizer";

@TargetClass(className = "java.beans.Introspector")
static final class Target_java_beans_Introspector {

// Do not load java.awt.Component and java.beans.Customizer classes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a doc comment.

// when they are not used
@Substitute
private static Class<?> findCustomizerClass(Class<?> type) {
String name = type.getName() + "Customizer";
try {
type = ClassFinder.findClass(name, type.getClassLoader());
if (Class.forName(COMPONENT_CLASS).isAssignableFrom(type)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot use Class.forName in this context because that would alter the behavior significantly: it would require a manual reflection configuration of those two classes, which no application would do properly and therefore code that actually relies on this method would no longer work properly.

The proper solution is a reachability handler: when java.awt.Component is reachable for any other reason, then a check for it is needed here too - but this code should not make it reachable itself. So we need a pattern similar to ProtectionDomainSupport/ProtectionDomainFeature: in a reachability handler for java.awt.Component, a non-final field Class<?> COMPONENT_CLASS is set to java.awt.Component.class. As long as that field is null, no isAssignableFrom check is done (and no check is necessary because that type is unreachable anyways).

But of course you found a propbably larger pattern where we currently include too much in the image: the problem is actually not that Component gets reachable - because a type being reachable has very little impact on image size - but that the static initializer of Component gets reachable. Currently we treat these two conditions as the same, but there is no good reason for that. We only need to make the static initializer reachable if a class could be initialized, and an instanceof check never triggers class initialization. I will think more about this and see if we can come up with a general solution that makes a substitution like this unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is the question which is not clear for me. The code in question is:

    private static Class<?> findCustomizerClass(Class<?> type) {
        String name = type.getName() + "Customizer";
        try {
            type = ClassFinder.findClass(name, type.getClassLoader());
            // Each customizer should inherit java.awt.Component and implement java.beans.Customizer
            // according to the section 9.3 of JavaBeans specification
            if (Component.class.isAssignableFrom(type) && Customizer.class.isAssignableFrom(type)) {
                ...

Is it possible for graal to understand that if a class Foo is passed to findCustomizerClass(...) method then the concatenation the Foo class name with "Customizer" string leads to a class which can't be found by ClassFinder.findClass(...) method (which is actually Class.forName() call with the given class loader)?

Or could you give more details about a reachability handler which can help in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot use Class.forName in this context because that would alter the behavior significantly: it would require a manual reflection configuration of those two classes, which no application would do properly and therefore code that actually relies on this method would no longer work properly.

For the code in question I see three use cases:

  1. Customizer class is not defined for the bean class
  2. Customizer class is properly defined (extends Component class and implements Customizer interface) for the bean class
  3. Customizer class is not properly defined (does not extend Component class or do not implement Customizer interface)

For the first case the application does not use the Component class at all and the behavior of running a program by java or native image gives the same result.

For the second case I used BeanSample example:

import java.beans.Introspector;

public class BeanSample {

    public static class BeanClass {
    }

    public static class BeanClassCustomizer extends java.awt.Component implements java.beans.Customizer {
        public void setObject(Object bean) {
            throw new UnsupportedOperationException();
        }
    }

    public static void main(String[] args) throws Exception {
        System.out.printf("load customizer for class: %s%n", BeanClass.class);
        Class<?> cls = Introspector.getBeanInfo(BeanClass.class).getBeanDescriptor().getCustomizerClass();
        System.out.printf("customizer: %s%n", cls);

    }
}

Java returns BeanSample$BeanClassCustomizer.

> java BeanSample
load customizer for class: class BeanSample$BeanClass
customizer: class BeanSample$BeanClassCustomizer

A native image which has been made without an additional configuration returns null customizer:

> native-image  --no-fallback BeanSample
> ./beansample
load customizer for class: class BeanSample$BeanClass
customizer: null

This is because the BeanClassCustomizer is only created by reflection.
It works if the reflect-config.json file is defined:

[
{
  "name":"BeanSample$BeanClass",
  "queryAllPublicMethods":true
},
{
  "name":"BeanSample$BeanClassCustomizer"
},
{
  "name":"java.beans.PropertyVetoException"
},
{
  "name":"java.lang.Object",
  "queryAllPublicMethods":true
}
]

Now the BeanSample$BeanClassCustomizer is returned:

> native-image --no-fallback -H:ReflectionConfigurationFiles=conf-dir/reflect-config.json BeanSample
> ./beansample
load customizer for class: class BeanSample$BeanClass
customizer: class BeanSample$BeanClassCustomizer

For the third case where the bean customizer does not extend Component class I used the BeanSample:

import java.beans.Introspector;

public class BeanSample {

    public static class BeanClass {
    }


    public static class BeanClassCustomizer {
    }

    public static void main(String[] args) throws Exception {
        System.out.printf("load customizer for class: %s%n", BeanClass.class);
        Class<?> cls = Introspector.getBeanInfo(BeanClass.class).getBeanDescriptor().getCustomizerClass();
        System.out.printf("customizer: %s%n", cls);

    }
}

Java returns null bean customizer:

> java BeanSample
load customizer for class: class BeanSample$BeanClass
customizer: null

The native image with reflection config file returns null customizer as well:

native-image --no-fallback --diagnostics-mode -H:ReflectionConfigurationFiles=conf-dir/reflect-config.json BeanSample
> ./beansample
load customizer for class: class BeanSample$BeanClass
customizer: null

As I see, for all three cases the result which is returned by Introspector.findCustomizerClass() method is the same.

For the fist case the Component class is not used at all.
For the second case the Component class is used by a beans customizer in a user program and can be derived by graal.
For the third case where a customizer does not extend the Component class the customizer class is handled in different ways in java ( the Component.class.isAssignableFrom(type) check fails) and in native image (ClassNotFoundException: java.awt.Component is thrown) but the result is the same because all exception are just ignored by Introspector.findCustomizerClass() method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that in the next release we plan to throw special runtime exceptions for missing metadata, so the try/catch block will not catch this:

#5171

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is updated to use the reachability handler for java.awt.Component class.

Two classes JavaBeansSupport/JavaBeansFeature are added in the similar way as ProtectionDomainSupport/ProtectionDomainFeature.

COMPONENT_CLASS is only set to java.awt.Component if it is reachable in the program.
Introspector.findCustomizerClass() substitution method returns null when COMPONENT_CLASS is not set because the customizer does not extend java.awt.Component in this case.

&& Class.forName(CUSTOMIZER_CLASS).isAssignableFrom(type)) {
return type;
}
} catch (Exception exception) {
// ignore any exceptions
}
return null;
}
}
// Checkstyle: resume
}