[GR-41096] Handle jdk.internal.module.ServicesCatalog.CLV correctly

PullRequest: graal/13035

Author: vjovanov

substratevm/CHANGELOG.md

@@ -11,6 +11,7 @@ This changelog summarizes major changes to GraalVM Native Image.
 * (GR-42148) Adjust build output to report types (primitives, classes, interfaces, and arrays) instead of classes and revise the output schema of `-H:BuildOutputJSONFile`.
 * (GR-42375) Add `-H:±GenerateBuildArtifactsFile` option, which generates a `build-artifacts.json` file with a list of all artifacts produced by Native Image. `.build_artifacts.txt` files are now deprecated, disabled (can be re-enabled with env setting `NATIVE_IMAGE_DEPRECATED_BUILD_ARTIFACTS_TXT=true`), and will be removed in a future release.
 * (GR-34179) Improved debugging support on Windows: Debug information now includes information about Java types (contributed by Red Hat).
+* (GR-41096) Support services loaded through the `java.util.ServiceLoader.ModuleServicesLookupIterator`. An example of such service is the `com.sun.jndi.rmi.registry.RegistryContextFactory`.
 
 ## Version 22.3.0
 * (GR-35721) Remove old build output style and the `-H:±BuildOutputUseNewStyle` option.

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JRTSupport.java

@@ -29,7 +29,9 @@
 import java.net.URL;
 import java.net.URLConnection;
 import java.nio.file.Path;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.BooleanSupplier;
@@ -42,6 +44,8 @@
 import com.oracle.svm.core.annotate.RecomputeFieldValue;
 import com.oracle.svm.core.annotate.Substitute;
 import com.oracle.svm.core.annotate.TargetClass;
+import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
+import com.oracle.svm.core.feature.InternalFeature;
 import com.oracle.svm.core.jdk.JRTSupport.JRTDisabled;
 import com.oracle.svm.core.jdk.JRTSupport.JRTEnabled;
 import com.oracle.svm.core.option.HostedOptionKey;
@@ -77,6 +81,21 @@ public boolean getAsBoolean() {
     }
 }
 
+@AutomaticallyRegisteredFeature
+class JRTDisableFeature implements InternalFeature {
+
+    @Override
+    public boolean isInConfiguration(IsInConfigurationAccess access) {
+        return !JRTSupport.Options.AllowJRTFileSystem.getValue();
+    }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    public void beforeAnalysis(BeforeAnalysisAccess access) {
+        ServiceCatalogSupport.singleton().removeServicesFromServicesCatalog("java.nio.file.spi.FileSystemProvider", new HashSet<>(Arrays.asList("jdk.internal.jrtfs.JrtFileSystemProvider")));
+    }
+}
+
 // region Enable jimage/jrtfs
 
 @TargetClass(className = "jdk.internal.jimage.ImageReader", innerClass = "SharedImageReader", onlyWith = JRTEnabled.class)

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JavaLangSubstitutions.java

@@ -31,6 +31,8 @@
 import java.io.File;
 import java.io.InputStream;
 import java.io.PrintStream;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.ConcurrentHashMap;
@@ -72,6 +74,10 @@
 import com.oracle.svm.core.snippets.SubstrateForeignCallTarget;
 import com.oracle.svm.core.thread.JavaThreads;
 import com.oracle.svm.core.util.VMError;
+import com.oracle.svm.util.ReflectionUtil;
+
+import jdk.internal.loader.ClassLoaderValue;
+import jdk.internal.module.ServicesCatalog;
 
 @TargetClass(java.lang.Object.class)
 @SuppressWarnings("static-method")
@@ -720,16 +726,42 @@ private static boolean hasClassPath() {
     }
 
     /**
-     * All ClassLoaderValue are reset at run time for now. See also
-     * {@link Target_java_lang_ClassLoader#classLoaderValueMap} for resetting of individual class
-     * loaders.
+     * Most {@link ClassLoaderValue}s are reset. For the list of preserved transformers see
+     * {@link ClassLoaderValueMapFieldValueTransformer}.
      */
     // Checkstyle: stop
-    @Alias @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.NewInstance, declClass = ConcurrentHashMap.class)//
+    @Alias @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Custom, declClass = ClassLoaderValueMapFieldValueTransformer.class, isFinal = true)//
     static ConcurrentHashMap<?, ?> CLASS_LOADER_VALUE_MAP;
     // Checkstyle: resume
 }
 
+final class ClassLoaderValueMapFieldValueTransformer implements FieldValueTransformer {
+    @Override
+    public Object transform(Object receiver, Object originalValue) {
+        if (originalValue == null) {
+            return null;
+        }
+
+        ConcurrentHashMap<?, ?> original = (ConcurrentHashMap<?, ?>) originalValue;
+        List<ClassLoaderValue<?>> clvs = Arrays.asList(
+                        ReflectionUtil.readField(ServicesCatalog.class, "CLV", null),
+                        ReflectionUtil.readField(ModuleLayer.class, "CLV", null));
+
+        var res = new ConcurrentHashMap<>();
+        for (ClassLoaderValue<?> clv : clvs) {
+            if (clv == null) {
+                throw VMError.shouldNotReachHere("Field must not be null. Please check what changed in the JDK.");
+            }
+            var catalog = original.get(clv);
+            if (catalog != null) {
+                res.put(clv, catalog);
+            }
+        }
+
+        return res;
+    }
+}
+
 /** Dummy class to have a class with the file's name. */
 public final class JavaLangSubstitutions {
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/ServiceCatalogSupport.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2023, 2023, 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 java.util.List;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
+import org.graalvm.nativeimage.ImageSingletons;
+import org.graalvm.nativeimage.Platform;
+import org.graalvm.nativeimage.Platforms;
+import org.graalvm.nativeimage.hosted.Feature;
+
+import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
+import com.oracle.svm.core.util.VMError;
+import com.oracle.svm.util.ReflectionUtil;
+
+import jdk.internal.module.ServicesCatalog;
+
+@AutomaticallyRegisteredImageSingleton
+@Platforms(Platform.HOSTED_ONLY.class)
+public class ServiceCatalogSupport {
+    final ConcurrentHashMap<String, Set<String>> omittedServiceProviders = new ConcurrentHashMap<>();
+    boolean sealed;
+
+    public static ServiceCatalogSupport singleton() {
+        return ImageSingletons.lookup(ServiceCatalogSupport.class);
+    }
+
+    public void seal() {
+        sealed = true;
+    }
+
+    public void removeServicesFromServicesCatalog(String serviceProvider, Set<String> services) {
+        VMError.guarantee(!sealed,
+                        "Removing services from a catalog is allowed only before analysis. ServiceCatalogSupport.removeServicesFromServicesCatalog called during or after analysis. ");
+        omittedServiceProviders.put(serviceProvider, services);
+    }
+
+    @SuppressWarnings("unchecked")
+    public void enableServiceCatalogMapTransformer(Feature.BeforeAnalysisAccess access) {
+        access.registerFieldValueTransformer(ReflectionUtil.lookupField(ServicesCatalog.class, "map"), (receiver, original) -> {
+            VMError.guarantee(sealed);
+            ConcurrentHashMap<String, List<ServicesCatalog.ServiceProvider>> map = (ConcurrentHashMap<String, List<ServicesCatalog.ServiceProvider>>) original;
+            final ConcurrentHashMap<String, List<ServicesCatalog.ServiceProvider>> res = new ConcurrentHashMap<>();
+            map.forEach((key, value) -> {
+                if (omittedServiceProviders.containsKey(key)) {
+                    var omittedServices = omittedServiceProviders.get(key);
+                    List<ServicesCatalog.ServiceProvider> filtered = value.stream()
+                                    .filter(v -> !omittedServices.contains(v.providerName()))
+                                    .collect(Collectors.toList());
+                    res.put(key, filtered);
+                } else {
+                    res.put(key, value);
+                }
+            });
+            return res;
+        });
+    }
+}

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_ClassLoader.java

@@ -36,6 +36,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Stream;
 
+import jdk.internal.loader.ClassLoaderValue;
 import org.graalvm.nativeimage.hosted.FieldValueTransformer;
 
 import com.oracle.svm.core.SubstrateUtil;
@@ -193,11 +194,10 @@ private Class<?> findLoadedClass0(String name) {
     }
 
     /**
-     * All ClassLoaderValue are reset at run time for now. See also
-     * {@link Target_jdk_internal_loader_BootLoader#CLASS_LOADER_VALUE_MAP} for resetting of the
-     * boot class loader.
+     * Most {@link ClassLoaderValue}s are reset. For the list of preserved transformers see
+     * {@link ClassLoaderValueMapFieldValueTransformer}.
      */
-    @Alias @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.NewInstance, declClass = ConcurrentHashMap.class)//
+    @Alias @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Custom, declClass = ClassLoaderValueMapFieldValueTransformer.class)//
     volatile ConcurrentHashMap<?, ?> classLoaderValueMap;
 
     /**

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_util_ServiceLoader.java

@@ -26,23 +26,13 @@
 package com.oracle.svm.core.jdk;
 
 import java.lang.reflect.Constructor;
-import java.net.URL;
 import java.security.AccessControlContext;
 import java.util.ArrayList;
-import java.util.Enumeration;
-import java.util.HashSet;
 import java.util.List;
-import java.util.ServiceConfigurationError;
-import java.util.ServiceLoader;
-import java.util.Set;
-
-import org.graalvm.compiler.core.common.SuppressFBWarnings;
 
 import com.oracle.svm.core.annotate.Alias;
 import com.oracle.svm.core.annotate.RecomputeFieldValue;
-import com.oracle.svm.core.annotate.Substitute;
 import com.oracle.svm.core.annotate.TargetClass;
-import com.oracle.svm.core.annotate.TargetElement;
 
 /**
  * Disable the module based iteration in favour of classpath based iteration. See
@@ -64,82 +54,6 @@ final class Target_java_util_ServiceLoader {
     private List<?> instantiatedProviders;
 }
 
-@TargetClass(value = java.util.ServiceLoader.class, innerClass = "ModuleServicesLookupIterator")
-final class Target_java_util_ServiceLoader_ModuleServicesLookupIterator {
-    @SuppressWarnings("unused")
-    @Substitute
-    Target_java_util_ServiceLoader_ModuleServicesLookupIterator(Target_java_util_ServiceLoader outer) {
-    }
-
-    @SuppressWarnings("static-method")
-    @Substitute
-    boolean hasNext() {
-        return false;
-    }
-}
-
-@TargetClass(value = java.util.ServiceLoader.class, innerClass = "LazyClassPathLookupIterator")
-final class Target_java_util_ServiceLoader_LazyClassPathLookupIterator {
-    @Alias//
-    ServiceLoader.Provider<?> nextProvider;
-    @Alias//
-    ServiceConfigurationError nextError;
-
-    /* Has to be initialized here, because we're substituting the constructor. */
-    @SuppressWarnings("unused")//
-    @Alias//
-    Set<String> providerNames = new HashSet<>();
-
-    /* Manage correct ref to the enclosing object ourselves so that we can access it. */
-    @Alias//
-    @TargetElement(name = "this$0")//
-    Target_java_util_ServiceLoader outer;
-
-    @Alias @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset)//
-    Enumeration<URL> configs;
-
-    @Substitute
-    Target_java_util_ServiceLoader_LazyClassPathLookupIterator(Target_java_util_ServiceLoader outer) {
-        this.outer = outer;
-    }
-
-    @Alias
-    private native Class<?> nextProviderClass();
-
-    /**
-     * Modified version of java.util.ServiceLoader.LazyClassPathLookupIterator#hasNextService.
-     */
-    @SuppressFBWarnings(value = "BC_IMPOSSIBLE_CAST", justification = "substitution hides acual type")
-    @Substitute
-    private boolean hasNextService() {
-        while (nextProvider == null && nextError == null) {
-            try {
-                Class<?> clazz = nextProviderClass();
-                if (clazz == null) {
-                    return false;
-                }
-
-                /*-
-                 * if (clazz.getModule().isNamed()) {
-                 *   // ignore class if in named module
-                 *  continue;
-                 *  }
-                 */
-
-                if (outer.service.isAssignableFrom(clazz)) {
-                    Constructor<?> ctor = outer.getConstructor(clazz);
-                    nextProvider = (ServiceLoader.Provider<?>) ((Object) (new Target_java_util_ServiceLoader_ProviderImpl(outer.service, clazz, ctor, outer.acc)));
-                } else {
-                    Target_java_util_ServiceLoader.fail(outer.service, clazz.getName() + " not a subtype");
-                }
-            } catch (ServiceConfigurationError e) {
-                nextError = e;
-            }
-        }
-        return true;
-    }
-}
-
 @TargetClass(value = java.util.ServiceLoader.class, innerClass = "ProviderImpl")
 final class Target_java_util_ServiceLoader_ProviderImpl {
 

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java

@@ -219,6 +219,7 @@
 import com.oracle.svm.core.hub.DynamicHub;
 import com.oracle.svm.core.hub.LayoutEncoding;
 import com.oracle.svm.core.image.ImageHeapLayouter;
+import com.oracle.svm.core.jdk.ServiceCatalogSupport;
 import com.oracle.svm.core.option.HostedOptionValues;
 import com.oracle.svm.core.option.RuntimeOptionValues;
 import com.oracle.svm.core.option.SubstrateOptionsParser;
@@ -750,7 +751,9 @@ protected boolean runPointsToAnalysis(String imageName, OptionValues options, De
         try (Indent ignored = debug.logAndIndent("run analysis")) {
             try (Indent ignored1 = debug.logAndIndent("process analysis initializers")) {
                 BeforeAnalysisAccessImpl config = new BeforeAnalysisAccessImpl(featureHandler, loader, bb, nativeLibraries, debug);
+                ServiceCatalogSupport.singleton().enableServiceCatalogMapTransformer(config);
                 featureHandler.forEachFeature(feature -> feature.beforeAnalysis(config));
+                ServiceCatalogSupport.singleton().seal();
                 bb.getHostVM().getClassInitializationSupport().setConfigurationSealed(true);
             }
 

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ServiceLoaderFeature.java

@@ -134,9 +134,7 @@ public static class Options {
     // before because implementation classes were instantiated using runtime reflection instead of
     // ServiceLoader (and thus weren't reachable in analysis).
 
-    protected final Set<String> serviceProvidersToSkip = new HashSet<>(Arrays.asList(
-                    "com.sun.jndi.rmi.registry.RegistryContextFactory"      // GR-26547
-    ));
+    protected final Set<String> serviceProvidersToSkip = new HashSet<>();
 
     /** Copy of private field {@code ServiceLoader.PREFIX}. */
     private static final String LOCATION_PREFIX = "META-INF/services/";
@@ -161,7 +159,6 @@ public boolean isInConfiguration(IsInConfigurationAccess access) {
 
     @Override
     public void afterRegistration(AfterRegistrationAccess access) {
-        // TODO write a more sophisticated include/exclude filter to handle cases like GR-27605 ?
         servicesToSkip.addAll(Options.ServiceLoaderFeatureExcludeServices.getValue().values());
         serviceProvidersToSkip.addAll(Options.ServiceLoaderFeatureExcludeServiceProviders.getValue().values());
     }

substratevm/src/com.oracle.svm.truffle.tck/src/com/oracle/svm/truffle/tck/resources/jre.json

@@ -238,5 +238,16 @@
         "justification": "In the JDK 17 Arrays#sort and Arrays#parallelSort share the same DualPivotQuicksort#sort(array,parallelism,low,high) implementation that uses ForkJoinPool for parallelism > 1. This causes a spurious privileged call being found in the Arrays#sort, so we allow Arrays#sort to avoid this false positive and keep Arrays#parallelSort denied."
       }
     ]
+  },
+  {  
+    "name": "java.util.ServiceLoader$ModuleServicesLookupIterator",
+    "methods": [
+      {
+        "name": "loaderFor",
+        "parameterTypes" : [
+          "java.lang.Module"
+        ]
+      }
+    ]
   }
 ]

truffle/mx.truffle/macro-truffle.properties

@@ -5,6 +5,9 @@ Args = -H:Features=com.oracle.svm.truffle.TruffleFeature,com.oracle.svm.truffle.
        --initialize-at-build-time=com.oracle.truffle \
        --initialize-at-build-time=org.graalvm.shadowed.org.jcodings \
        --initialize-at-build-time=com.oracle.truffle.tools.utils.json \
-       --initialize-at-build-time=org.graalvm.shadowed.org.jline,org.graalvm.shadowed.org.fusesource.jansi
+       --initialize-at-build-time=org.graalvm.shadowed.org.jline,org.graalvm.shadowed.org.fusesource.jansi \
+       --initialize-at-run-time=sun.rmi \
+       --initialize-at-run-time=java.rmi
+
 JavaArgs = -Dtruffle.TruffleRuntime=com.oracle.svm.truffle.api.SubstrateTruffleRuntime \
            -Dgraalvm.ForcePolyglotInvalid=false