diff --git a/substratevm/mx.substratevm/mx_substratevm.py b/substratevm/mx.substratevm/mx_substratevm.py index f66baf87934b..9c24fef71581 100644 --- a/substratevm/mx.substratevm/mx_substratevm.py +++ b/substratevm/mx.substratevm/mx_substratevm.py @@ -334,7 +334,7 @@ def native_image_func(args, **kwargs): yield native_image_func native_image_context.hosted_assertions = ['-J-ea', '-J-esa'] -_native_unittest_features = '--features=com.oracle.svm.test.ImageInfoTest$TestFeature,com.oracle.svm.test.ServiceLoaderTest$TestFeature,com.oracle.svm.test.SecurityServiceTest$TestFeature,com.oracle.svm.test.ReflectionRegistrationTest$TestFeature' +_native_unittest_features = '--features=com.oracle.svm.test.ImageInfoTest$TestFeature,com.oracle.svm.test.services.ServiceLoaderTest$TestFeature,com.oracle.svm.test.services.SecurityServiceTest$TestFeature,com.oracle.svm.test.ReflectionRegistrationTest$TestFeature' IMAGE_ASSERTION_FLAGS = svm_experimental_options(['-H:+VerifyGraalGraphs', '-H:+VerifyPhases']) @@ -545,8 +545,8 @@ def native_unittests_task(extra_build_args=None): out.write(f"Simple file{i}" + '\n') additional_build_args = svm_experimental_options([ - '-H:AdditionalSecurityProviders=com.oracle.svm.test.SecurityServiceTest$NoOpProvider', - '-H:AdditionalSecurityServiceTypes=com.oracle.svm.test.SecurityServiceTest$JCACompliantNoOpService', + '-H:AdditionalSecurityProviders=com.oracle.svm.test.services.SecurityServiceTest$NoOpProvider', + '-H:AdditionalSecurityServiceTypes=com.oracle.svm.test.services.SecurityServiceTest$JCACompliantNoOpService', '-cp', cp_entry_name ]) if extra_build_args is not None: diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ServiceLoaderFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ServiceLoaderFeature.java index 690ff5d5421d..68adeb8a6745 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ServiceLoaderFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ServiceLoaderFeature.java @@ -41,8 +41,8 @@ import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature; import com.oracle.svm.core.feature.InternalFeature; import com.oracle.svm.core.jdk.ServiceCatalogSupport; -import com.oracle.svm.core.option.HostedOptionKey; import com.oracle.svm.core.option.AccumulatingLocatableMultiOptionValue; +import com.oracle.svm.core.option.HostedOptionKey; import com.oracle.svm.hosted.analysis.Inflation; import jdk.graal.compiler.options.Option; @@ -218,7 +218,21 @@ void handleServiceClassIsReachable(DuringAnalysisAccess access, Class service if (nullaryConstructor != null || nullaryProviderMethod != null) { RuntimeReflection.register(providerClass); if (nullaryConstructor != null) { + /* + * Registering a constructor with + * RuntimeReflection.registerConstructorLookup(providerClass) does not produce + * the same behavior as using RuntimeReflection.register(nullaryConstructor). In + * the first case, the constructor is marked for query purposes only, so this + * if-statement cannot be eliminated. + * + */ RuntimeReflection.register(nullaryConstructor); + } else { + /* + * If there's no nullary constructor, register it as negative lookup to avoid + * throwing a MissingReflectionRegistrationError at run time. + */ + RuntimeReflection.registerConstructorLookup(providerClass); } if (nullaryProviderMethod != null) { RuntimeReflection.register(nullaryProviderMethod); @@ -229,8 +243,14 @@ void handleServiceClassIsReachable(DuringAnalysisAccess access, Class service */ RuntimeReflection.registerMethodLookup(providerClass, "provider"); } - registeredProviders.add(provider); } + /* + * Register the provider in both cases: when it is JCA-compliant (has a nullary + * constructor or a provider method) or when it lacks both. If neither is present, a + * ServiceConfigurationError will be thrown at runtime, consistent with HotSpot + * behavior. + */ + registeredProviders.add(provider); } if (!registeredProviders.isEmpty()) { String serviceResourceLocation = "META-INF/services/" + serviceProvider.getName(); diff --git a/substratevm/src/com.oracle.svm.test/src/META-INF/native-image/com.oracle.svm.test/native-image.properties b/substratevm/src/com.oracle.svm.test/src/META-INF/native-image/com.oracle.svm.test/native-image.properties index 53830eac03f2..4f50bea8efba 100644 --- a/substratevm/src/com.oracle.svm.test/src/META-INF/native-image/com.oracle.svm.test/native-image.properties +++ b/substratevm/src/com.oracle.svm.test/src/META-INF/native-image/com.oracle.svm.test/native-image.properties @@ -2,8 +2,8 @@ Args= \ --initialize-at-run-time=com.oracle.svm.test \ --initialize-at-build-time=com.oracle.svm.test.AbstractClassSerializationTest,com.oracle.svm.test.SerializationRegistrationTest,com.oracle.svm.test.SerializationRegistrationTest$SerializableTestClass,com.oracle.svm.test.SerializationRegistrationTest$AssociatedRegistrationTestClass,com.oracle.svm.test.LambdaClassSerializationTest,com.oracle.svm.test.LambdaClassDeserializationTest \ --features=com.oracle.svm.test.SerializationRegistrationTestFeature \ - --features=com.oracle.svm.test.AbstractServiceLoaderTest$TestFeature \ - --features=com.oracle.svm.test.NoProviderConstructorServiceLoaderTest$TestFeature \ + --features=com.oracle.svm.test.services.AbstractServiceLoaderTest$TestFeature \ + --features=com.oracle.svm.test.services.NoProviderConstructorServiceLoaderTest$TestFeature \ --features=com.oracle.svm.test.NativeImageResourceUtils$TestFeature \ --features=com.oracle.svm.test.jfr.JfrTestFeature \ --add-opens=java.base/java.lang=ALL-UNNAMED \ diff --git a/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.AbstractServiceLoaderTest$ServiceInterface b/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.AbstractServiceLoaderTest$ServiceInterface deleted file mode 100644 index 2a453fda35a1..000000000000 --- a/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.AbstractServiceLoaderTest$ServiceInterface +++ /dev/null @@ -1,2 +0,0 @@ -com.oracle.svm.test.AbstractServiceLoaderTest$ConcreteService -com.oracle.svm.test.AbstractServiceLoaderTest$AbstractService diff --git a/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.NoProviderConstructorServiceLoaderTest$ServiceInterface b/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.NoProviderConstructorServiceLoaderTest$ServiceInterface deleted file mode 100644 index b1a8b1378090..000000000000 --- a/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.NoProviderConstructorServiceLoaderTest$ServiceInterface +++ /dev/null @@ -1,2 +0,0 @@ -com.oracle.svm.test.NoProviderConstructorServiceLoaderTest$ProperService -com.oracle.svm.test.NoProviderConstructorServiceLoaderTest$NoProviderConstructorService diff --git a/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.ServiceLoaderTest$ServiceInterface b/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.ServiceLoaderTest$ServiceInterface deleted file mode 100644 index def0bddd5a3f..000000000000 --- a/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.ServiceLoaderTest$ServiceInterface +++ /dev/null @@ -1,3 +0,0 @@ -com.oracle.svm.test.ServiceLoaderTest$ServiceA -com.oracle.svm.test.ServiceLoaderTest$ServiceB -com.oracle.svm.test.ServiceLoaderTest$ServiceHostedOnly diff --git a/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.services.AbstractServiceLoaderTest$ServiceInterface b/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.services.AbstractServiceLoaderTest$ServiceInterface new file mode 100644 index 000000000000..67e9f89502b1 --- /dev/null +++ b/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.services.AbstractServiceLoaderTest$ServiceInterface @@ -0,0 +1,2 @@ +com.oracle.svm.test.services.AbstractServiceLoaderTest$ConcreteService +com.oracle.svm.test.services.AbstractServiceLoaderTest$AbstractService diff --git a/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.services.NoProviderConstructorServiceLoaderTest$ServiceInterface b/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.services.NoProviderConstructorServiceLoaderTest$ServiceInterface new file mode 100644 index 000000000000..3aef4f141c57 --- /dev/null +++ b/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.services.NoProviderConstructorServiceLoaderTest$ServiceInterface @@ -0,0 +1,2 @@ +com.oracle.svm.test.services.NoProviderConstructorServiceLoaderTest$ProperService +com.oracle.svm.test.services.NoProviderConstructorServiceLoaderTest$NoProviderConstructorService diff --git a/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.services.ServiceLoaderTest$ServiceInterface b/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.services.ServiceLoaderTest$ServiceInterface new file mode 100644 index 000000000000..405b64e3b310 --- /dev/null +++ b/substratevm/src/com.oracle.svm.test/src/META-INF/services/com.oracle.svm.test.services.ServiceLoaderTest$ServiceInterface @@ -0,0 +1,3 @@ +com.oracle.svm.test.services.ServiceLoaderTest$ServiceA +com.oracle.svm.test.services.ServiceLoaderTest$ServiceB +com.oracle.svm.test.services.ServiceLoaderTest$ServiceHostedOnly diff --git a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/AbstractServiceLoaderTest.java b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/AbstractServiceLoaderTest.java similarity index 99% rename from substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/AbstractServiceLoaderTest.java rename to substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/AbstractServiceLoaderTest.java index 19868f01c643..aba3ca47feb8 100644 --- a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/AbstractServiceLoaderTest.java +++ b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/AbstractServiceLoaderTest.java @@ -22,7 +22,7 @@ * or visit www.oracle.com if you need additional information or have any * questions. */ -package com.oracle.svm.test; +package com.oracle.svm.test.services; import java.util.HashSet; import java.util.ServiceConfigurationError; diff --git a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/NoProviderConstructorServiceLoaderTest.java b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/NoProviderConstructorServiceLoaderTest.java similarity index 59% rename from substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/NoProviderConstructorServiceLoaderTest.java rename to substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/NoProviderConstructorServiceLoaderTest.java index aa2f328a817d..22532a34eb94 100644 --- a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/NoProviderConstructorServiceLoaderTest.java +++ b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/NoProviderConstructorServiceLoaderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2024, 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 @@ -22,7 +22,7 @@ * or visit www.oracle.com if you need additional information or have any * questions. */ -package com.oracle.svm.test; +package com.oracle.svm.test.services; import java.util.HashSet; import java.util.ServiceConfigurationError; @@ -40,12 +40,8 @@ // Checkstyle: allow Class.getSimpleName /** - * Tests a workaround for {@linkplain ServiceLoader services} without a {@linkplain ServiceLoader - * provider constructor} (nullary constructor) [GR-19958]. The workaround completely ignores - * services without a provider constructor, instead of throwing an {@link ServiceConfigurationError} - * when iterating the services. See the Github issue - * "Spring Service Registry native-image - * failure due to service loader handling in jersey #2652" for more details. + * Test both JCA-compliant services and non-JCA-compliant services (without a nullary constructor), + * and compare the behavior between Native Image and Hotspot. */ public class NoProviderConstructorServiceLoaderTest { @@ -73,81 +69,77 @@ public abstract static class NoProviderConstructorService implements ServiceInte private static final Set EXPECTED = Set.of(ProperService.class.getSimpleName()); - /** - * This should actually throw an {@link ServiceConfigurationError}. - * - * @see #testLazyStreamHotspot() - */ - @Test + @Test(expected = ServiceConfigurationError.class) public void testLazyStreamNativeImage() { - Assume.assumeTrue("native image specific behavior", ImageInfo.inImageRuntimeCode()); - Set simpleNames = ServiceLoader.load(ServiceInterface.class).stream() - .map(provider -> provider.type().getSimpleName()) - .collect(Collectors.toSet()); + assumeEnvironment(true); + Set simpleNames = loadLazyStreamNames(); Assert.assertEquals(EXPECTED, simpleNames); } - /** - * This should actually throw an {@link ServiceConfigurationError}. - * - * @see #testEagerStreamHotspot() - */ - @Test + @Test(expected = ServiceConfigurationError.class) public void testEagerStreamNativeImage() { - Assume.assumeTrue("native image specific behavior", ImageInfo.inImageRuntimeCode()); - Set simpleNames = ServiceLoader.load(ServiceInterface.class).stream() - .map(provider -> provider.get().getClass().getSimpleName()) - .collect(Collectors.toSet()); + assumeEnvironment(true); + Set simpleNames = loadEagerStreamNames(); Assert.assertEquals(EXPECTED, simpleNames); } - /** - * This should actually throw an {@link ServiceConfigurationError}. - * - * @see #testEagerIteratorHotspot() - */ - @Test + @Test(expected = ServiceConfigurationError.class) public void testEagerIteratorNativeImage() { - Assume.assumeTrue("native image specific behavior", ImageInfo.inImageRuntimeCode()); - Set simpleNames = new HashSet<>(); - ServiceLoader.load(ServiceInterface.class).iterator() - .forEachRemaining(s -> simpleNames.add(s.getClass().getSimpleName())); + assumeEnvironment(true); + Set simpleNames = loadEagerIteratorNames(); Assert.assertEquals(EXPECTED, simpleNames); } - /** - * @see #testLazyStreamNativeImage() - */ @Test(expected = ServiceConfigurationError.class) public void testLazyStreamHotspot() { - Assume.assumeFalse("hotspot specific behavior", ImageInfo.inImageRuntimeCode()); - Set simpleNames = ServiceLoader.load(ServiceInterface.class).stream() + assumeEnvironment(false); + Set simpleNames = loadLazyStreamNames(); + Assert.assertNull("should not reach", simpleNames); + } + + @Test(expected = ServiceConfigurationError.class) + public void testEagerIteratorHotspot() { + assumeEnvironment(false); + Set simpleNames = loadEagerIteratorNames(); + Assert.assertNull("should not reach", simpleNames); + } + + /** + * Helper method to assume the environment (hotspot/native image). + */ + private static void assumeEnvironment(boolean isNativeImage) { + if (isNativeImage) { + Assume.assumeTrue("native image specific behavior", ImageInfo.inImageRuntimeCode()); + } else { + Assume.assumeFalse("hotspot specific behavior", ImageInfo.inImageRuntimeCode()); + } + } + + /** + * Helper method for lazy stream tests. + */ + private static Set loadLazyStreamNames() { + return ServiceLoader.load(ServiceInterface.class).stream() .map(provider -> provider.type().getSimpleName()) .collect(Collectors.toSet()); - Assert.assertNull("should not reach", simpleNames); } /** - * @see #testEagerStreamNativeImage() + * Helper method for eager stream tests. */ - @Test(expected = ServiceConfigurationError.class) - public void testEagerStreamHotspot() { - Assume.assumeFalse("hotspot specific behavior", ImageInfo.inImageRuntimeCode()); - Set simpleNames = ServiceLoader.load(ServiceInterface.class).stream() + private static Set loadEagerStreamNames() { + return ServiceLoader.load(ServiceInterface.class).stream() .map(provider -> provider.get().getClass().getSimpleName()) .collect(Collectors.toSet()); - Assert.assertNull("should not reach", simpleNames); } /** - * @see #testEagerIteratorNativeImage() + * Helper method for eager iterator tests. */ - @Test(expected = ServiceConfigurationError.class) - public void testEagerIteratorHotspot() { - Assume.assumeFalse("hotspot specific behavior", ImageInfo.inImageRuntimeCode()); + private static Set loadEagerIteratorNames() { Set simpleNames = new HashSet<>(); ServiceLoader.load(ServiceInterface.class).iterator() .forEachRemaining(s -> simpleNames.add(s.getClass().getSimpleName())); - Assert.assertNull("should not reach", simpleNames); + return simpleNames; } } diff --git a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/SecurityServiceTest.java b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/SecurityServiceTest.java similarity index 99% rename from substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/SecurityServiceTest.java rename to substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/SecurityServiceTest.java index 46393bf2bdc1..488ebc3a69ec 100644 --- a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/SecurityServiceTest.java +++ b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/SecurityServiceTest.java @@ -22,7 +22,7 @@ * or visit www.oracle.com if you need additional information or have any * questions. */ -package com.oracle.svm.test; +package com.oracle.svm.test.services; import java.security.NoSuchAlgorithmException; import java.security.Provider; diff --git a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/ServiceLoaderTest.java b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/ServiceLoaderTest.java similarity index 99% rename from substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/ServiceLoaderTest.java rename to substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/ServiceLoaderTest.java index b74b429700b9..a0ef7524a648 100644 --- a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/ServiceLoaderTest.java +++ b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/services/ServiceLoaderTest.java @@ -22,7 +22,7 @@ * or visit www.oracle.com if you need additional information or have any * questions. */ -package com.oracle.svm.test; +package com.oracle.svm.test.services; import java.util.ArrayList; import java.util.List;