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

Add known service providers that are not found through module lookup #5985

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

koutheir
Copy link
Contributor

Add known service providers that are not found through module lookup.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Feb 13, 2023
@fniephaus
Copy link
Member

Why are these service provides "not found through module lookup"? Do you have an example app that needs this patch or maybe even a test that we could add for this?

@koutheir
Copy link
Contributor Author

Why are these service provides "not found through module lookup"?

Because the replacement implementation of service loader does not handle services provided inside modules.

@fniephaus
Copy link
Member

the replacement implementation of service loader does not handle services provided inside modules.

I'm not familiar with that part of the JDK, but can we fix the root cause rather than the symptoms?

@koutheir
Copy link
Contributor Author

The root cause is not in the JDK. The JDK implementation works fine. The root cause is in the rudimentary replacement implementation of that system in Graal, which only supports services loaded through the class path, but not through the module system. Adding support for that to Graal is a major redesigning task, whereas marking well-known JDK implementations of services is a fast way to make them work.

@fniephaus
Copy link
Member

Adding support for that to Graal is a major redesigning task, whereas marking well-known JDK implementations of services is a fast way to make them work.

I believe adding support for this is better than starting to hard-code service registrations. We'll take another look, support for the module system has significantly been improved over the last couple of months.

@olpaw
Copy link
Member

olpaw commented Feb 15, 2023

As an intermediate solution this is acceptable. But we should make it high priority to make this change obsolete by implementing proper module-aware serviceProviders lookup in com.oracle.svm.hosted.ServiceLoaderFeature.

@koutheir
Copy link
Contributor Author

I believe adding support for this is better than starting to hard-code service registrations.

I agree.

We'll take another look, support for the module system has significantly been improved over the last couple of months.

For reference, the code base I'm working with is GraalVM EE 22.3.1.

@olpaw
Copy link
Member

olpaw commented Feb 16, 2023

Hi @koutheir

The root cause is not in the JDK. The JDK implementation works fine.

are you sure that the ServiceProviders you are trying to add here are working as expected on JVM?

Consider the following example:

import javax.xml.datatype.DatatypeConfigurationException;
import javax.xml.datatype.DatatypeFactory;
import java.util.ServiceLoader;

public class App {
    public static void main(String[] args) throws DatatypeConfigurationException {
        DatatypeFactory instance = DatatypeFactory.newInstance();
        System.out.println("DatatypeFactory via DatatypeFactory.newInstance():");
        System.out.println(instance.getClass().getName());
        System.out.println("DatatypeFactory via ServiceLoader.load(DatatypeFactory.class):");
        ServiceLoader<DatatypeFactory> serviceLoader = ServiceLoader.load(DatatypeFactory.class);
        serviceLoader.forEach(datatypeFactory -> {
            System.out.println(datatypeFactory.getClass().getName());
        });
    }
}

if I run this with java I get:

$ ~/.local/graalvm/graalvm-ee-java19-22.3.1/bin/java -jar target/app-1.0-SNAPSHOT.jar
DatatypeFactory via DatatypeFactory.newInstance():
com.sun.org.apache.xerces.internal.jaxp.datatype.DatatypeFactoryImpl
DatatypeFactory via ServiceLoader.load(DatatypeFactory.class):

As you can see if I instantiate DatatypeFactory via the intended factory method I do get com.sun.org.apache.xerces.internal.jaxp.datatype.DatatypeFactoryImpl just fine, but if I try to do the same via ServiceLoader I get nothing. Also looking at the module-info.class of module java.xml does not indicate that DatatypeFactoryImpl is registered as service provider (no provides…with for DatatypeFactoryImpl).

I'm using the same version of GraalVM as the one you reported:

$ ~/.local/graalvm/graalvm-ee-java19-22.3.1/bin/java --version
java 19.0.2 2023-01-17
Java(TM) SE Runtime Environment GraalVM EE 22.3.1 (build 19.0.2+7-jvmci-22.3-b11)
Java HotSpot(TM) 64-Bit Server VM GraalVM EE 22.3.1 (build 19.0.2+7-jvmci-22.3-b11, mixed mode, sharing)

Please provide a reproducer that demonstrates that DatatypeFactoryImpl can indeed be instantiated via the ServiceLoader mechanism.

Copy link
Member

@olpaw olpaw left a comment

Choose a reason for hiding this comment

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

@koutheir
Copy link
Contributor Author

koutheir commented Feb 20, 2023

Here is a sample (Hello.java) that reproduces the issue and behaves differently under JDK 11 compared to native-image from Graal EE 22.3.1.

// Hello.java
// Requires META-INF/services/javax.xml.parsers.DocumentBuilderFactory

import java.net.URL;
import java.net.URLClassLoader;
import java.util.HashSet;
import java.util.Iterator;
import java.util.ServiceLoader;
import java.util.Set;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.ParserConfigurationException;

public class Hello extends DocumentBuilderFactory {
    public static void main(String[] args) throws Exception {
        DocumentBuilderFactory instance = DocumentBuilderFactory.newInstance();
        String instanceClassName = instance.getClass().getName();

        ServiceLoader<DocumentBuilderFactory> sl = ServiceLoader.load(DocumentBuilderFactory.class);
        Set<String> implNames = new HashSet<String>();
        Iterator<DocumentBuilderFactory> iter = sl.iterator();
        while (iter.hasNext()) {
            String implName = iter.next().getClass().getName();
            implNames.add(implName);
        }

        System.out.println(implNames.contains(instanceClassName) ? "PASSED" : "FAILED");
    }

    @Override
    public Object getAttribute(String name) throws IllegalArgumentException {
        return null;
    }

    @Override
    public DocumentBuilder newDocumentBuilder() throws ParserConfigurationException {
        return null;
    }

    @Override
    public void setAttribute(String name, Object value) throws IllegalArgumentException {

    }

    @Override
    public void setFeature(String name, boolean value) throws ParserConfigurationException {

    }

    @Override
    public boolean getFeature(String name) throws ParserConfigurationException {
        return false;
    }
}

This sample needs the additional file META-INF/services/javax.xml.parsers.DocumentBuilderFactory containing the following:

Hello

Running the sample with JDK 11 produces PASSED as output. Running it as a native image, without the changes introduced in this PR, produces the output FAILED. Running it as a native image, patched with the changes introduced in this PR, produces the output PASSED.

@koutheir koutheir requested a review from olpaw February 20, 2023 18:07
@olpaw
Copy link
Member

olpaw commented Feb 20, 2023

@koutheir your example works perfectly fine with native-image from GraalVM 22.3.1 Java 11 EE:

pwoegere@olpaw:~/OLabs$ unzip reproducer.zip 
Archive:  reproducer.zip
   creating: reproducer/
  inflating: reproducer/Hello.java   
   creating: reproducer/META-INF/
   creating: reproducer/META-INF/services/
  inflating: reproducer/META-INF/services/javax.xml.parsers.DocumentBuilderFactory  
pwoegere@olpaw:~/OLabs$ cd reproducer/
pwoegere@olpaw:~/OLabs/reproducer$ ~/.local/graalvm/graalvm-ee-java11-22.3.1/bin/native-image --version
GraalVM 22.3.1 Java 11 EE (Java Version 11.0.18+9-LTS-jvmci-22.3-b11)
pwoegere@olpaw:~/OLabs/reproducer$ ~/.local/graalvm/graalvm-ee-java11-22.3.1/bin/java --version
java 11.0.18 2023-01-17 LTS
Java(TM) SE Runtime Environment GraalVM EE 22.3.1 (build 11.0.18+9-LTS-jvmci-22.3-b11)
Java HotSpot(TM) 64-Bit Server VM GraalVM EE 22.3.1 (build 11.0.18+9-LTS-jvmci-22.3-b11, mixed mode, sharing)
pwoegere@olpaw:~/OLabs/reproducer$ ~/.local/graalvm/graalvm-ee-java11-22.3.1/bin/javac Hello.java 
pwoegere@olpaw:~/OLabs/reproducer$ ~/.local/graalvm/graalvm-ee-java11-22.3.1/bin/java Hello 
PASSED
pwoegere@olpaw:~/OLabs/reproducer$ ~/.local/graalvm/graalvm-ee-java11-22.3.1/bin/native-image Hello 
========================================================================================================================
GraalVM Native Image: Generating 'hello' (executable)...
========================================================================================================================
[1/7] Initializing...                                                                                    (2.8s @ 0.24GB)
 Version info: 'GraalVM 22.3.1 Java 11 EE'
 Java version info: '11.0.18+9-LTS-jvmci-22.3-b11'
 C compiler: gcc (redhat, x86_64, 12.2.1)
 Garbage collector: Serial GC
[2/7] Performing analysis...  [*****]                                                                    (4.9s @ 1.14GB)
   2,807 (71.24%) of  3,940 classes reachable
   3,492 (53.02%) of  6,586 fields reachable
  13,118 (45.20%) of 29,024 methods reachable
      29 classes,     0 fields, and   268 methods registered for reflection
      58 classes,    58 fields, and    52 methods registered for JNI access
       4 native libraries: dl, pthread, rt, z
[3/7] Building universe...                                                                               (0.7s @ 1.60GB)
[4/7] Parsing methods...      [*]                                                                        (0.6s @ 0.80GB)
[5/7] Inlining methods...     [***]                                                                      (0.3s @ 1.15GB)
[6/7] Compiling methods...    [***]                                                                      (8.3s @ 4.38GB)
[7/7] Creating image...                                                                                  (1.3s @ 0.42GB)
   5.04MB (44.86%) for code area:     6,481 compilation units
   5.96MB (53.06%) for image heap:  106,678 objects and 6 resources
 238.59KB ( 2.07%) for other data
  11.24MB in total
------------------------------------------------------------------------------------------------------------------------
Top 10 packages in code area:                               Top 10 object types in image heap:
 694.58KB java.util                                          982.35KB byte[] for code metadata
 398.52KB java.lang                                          728.47KB byte[] for general heap data
 390.07KB java.text                                          702.16KB java.lang.String
 277.28KB com.oracle.svm.core.code                           634.02KB byte[] for java.lang.String
 213.04KB java.util.concurrent                               436.93KB java.lang.Class
 196.58KB java.util.regex                                    301.38KB java.util.HashMap$Node
 148.89KB java.util.logging                                  194.31KB java.util.concurrent.ConcurrentHashMap$Node
 141.67KB com.oracle.svm.core.genscavenge                    139.54KB byte[] for embedded resources
 137.11KB java.math                                          137.28KB char[]
 133.02KB java.io                                            136.16KB java.util.HashMap$Node[]
   2.33MB for 125 more packages                                1.16MB for 763 more object types
------------------------------------------------------------------------------------------------------------------------
                        0.5s (2.4% of total time) in 21 GCs | Peak RSS: 5.38GB | CPU load: 10.51
------------------------------------------------------------------------------------------------------------------------
Produced artifacts:
 /home/pwoegere/OLabs/reproducer/hello (executable)
 /home/pwoegere/OLabs/reproducer/hello.build_artifacts.txt (txt)
========================================================================================================================
Finished generating 'hello' in 20.0s.
pwoegere@olpaw:~/OLabs/reproducer$ ls -la
total 11392
drwxr-xr-x 1 pwoegere pwoegere      118 Feb 20 20:24 .
drwxrwxr-x 1 pwoegere pwoegere      456 Feb 20 20:22 ..
-rwxr-xr-x 1 pwoegere pwoegere 11652040 Feb 20 20:24 hello
-rw-r--r-- 1 pwoegere pwoegere       20 Feb 20 20:24 hello.build_artifacts.txt
-rw-r--r-- 1 pwoegere pwoegere     1878 Feb 20 20:23 Hello.class
-rw-r--r-- 1 pwoegere pwoegere     1715 Feb 20 20:17 Hello.java
drwxr-xr-x 1 pwoegere pwoegere       16 Feb 20 20:18 META-INF
pwoegere@olpaw:~/OLabs/reproducer$ ./hello 
PASSED

@olpaw
Copy link
Member

olpaw commented Feb 20, 2023

reproducer.zip

@olpaw
Copy link
Member

olpaw commented Feb 22, 2023

@koutheir I think I know know what is actually going on here.

All those failing ServiceLoader related tests install a custom URLClassLoader (see ServiceLoaderVerifierBase#createClassLoader) (with a URL that contains a META-INF/services/MyServiceProvider subdir and an implementation class for that Service) as the ContextClassLoader of the current thread (see ServiceLoaderVerifierBase#perform). Then later during test execution when java.util.ServiceLoader#load(java.lang.Class<S>) gets called, it uses that custom URLClassLoader to resolve the ServiceProvider.

    @CallerSensitive
    public static <S> ServiceLoader<S> load(Class<S> service) {
        ClassLoader cl = Thread.currentThread().getContextClassLoader(); <= Returns the custom URLClassLoader
        return new ServiceLoader<>(Reflection.getCallerClass(), service, cl);
    }

I can see how this will fail when ran as image. This class of tests performs classloading at image runtime. I will extend my example so that it verifies my assumption. Then I will think about how we could make this working with the help of the agent.

@koutheir
Copy link
Contributor Author

That is also what my analysis concluded so far. In my sample, I avoided the class loading part though, but that might have hidden the failure somehow.

@olpaw
Copy link
Member

olpaw commented Feb 22, 2023

reproducer.zip

This is the working reproducer for the issue.

  • Unzip & cd into reproducer/default
  • Run java App
    (compiled with JDK 17 but could also be compiled with 11)

Currently we are not able to build such code into a working native-image.

The changes proposed by this PR are unfortunately not suitable to solve the issue.

Maybe with the tracing agent we can track URLClassloader instantiations and their URLs. Then in combination with resource access tracking and predefined classes support we can - maybe - get this working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants