Skip to content

Commit

Permalink
[GR-53449] Do not register all declared methods of service provider c…
Browse files Browse the repository at this point in the history 
…lasses for reflection.

PullRequest: graal/17479
  • Loading branch information
@woess
woess committed Apr 26, 2024
2 parents 9f374b2 + 23f22dc commit b26cb04
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 23 deletions.
52 changes: 39 additions & 13 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 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
Expand Down Expand Up @@ -1146,17 +1146,30 @@ private Method getMethod(String methodName, Class<?>... parameterTypes) throws N
}

private void checkMethod(String methodName, Class<?>[] parameterTypes, Method method, boolean publicOnly) throws NoSuchMethodException {
if (CONSTRUCTOR_NAME.equals(methodName)) {
if (!checkMethodExists(methodName, parameterTypes, method, publicOnly)) {
throw new NoSuchMethodException(methodToString(methodName, parameterTypes));
}
checkExecutable(methodName, parameterTypes, method, publicOnly);
}

private boolean checkMethodExists(String methodName, Class<?>[] parameterTypes, Method method, boolean publicOnly) {
if (CONSTRUCTOR_NAME.equals(methodName)) {
return false;
}
return checkExecutableExists(methodName, parameterTypes, method, publicOnly);
}

private void checkConstructor(Class<?>[] parameterTypes, Constructor<?> constructor, boolean publicOnly) throws NoSuchMethodException {
checkExecutable(CONSTRUCTOR_NAME, parameterTypes, constructor, publicOnly);
if (!checkExecutableExists(CONSTRUCTOR_NAME, parameterTypes, constructor, publicOnly)) {
throw new NoSuchMethodException(methodToString(CONSTRUCTOR_NAME, parameterTypes));
}
}

private void checkExecutable(String methodName, Class<?>[] parameterTypes, Executable method, boolean publicOnly) throws NoSuchMethodException {
/**
* Checks if the method exists and reports any missing reflection registration errors.
*
* @return true if the method exists and is visible, false if missing (NoSuchMethodException).
*/
private boolean checkExecutableExists(String methodName, Class<?>[] parameterTypes, Executable method, boolean publicOnly) {
boolean throwMissingErrors = throwMissingRegistrationErrors();
Class<?> clazz = DynamicHub.toClass(this);
if (method == null) {
Expand All @@ -1172,7 +1185,7 @@ private void checkExecutable(String methodName, Class<?>[] parameterTypes, Execu
* sure that the method does indeed not exist if we don't find it. This is also the case
* when querying an interface constructor.
*/
throw new NoSuchMethodException(methodToString(methodName, parameterTypes));
return false;
} else {
RuntimeMetadataDecoder decoder = ImageSingletons.lookup(RuntimeMetadataDecoder.class);
int methodModifiers = method.getModifiers();
Expand All @@ -1181,9 +1194,7 @@ private void checkExecutable(String methodName, Class<?>[] parameterTypes, Execu
if (throwMissingErrors && hiding) {
MissingReflectionRegistrationUtils.forMethod(clazz, methodName, parameterTypes);
}
if (negative || hiding) {
throw new NoSuchMethodException(methodToString(methodName, parameterTypes));
}
return !(negative || hiding);
}
}

Expand Down Expand Up @@ -1597,21 +1608,36 @@ private String getSimpleBinaryName0() {
}

/**
* Used by {@link java.util.ServiceLoader} to find any {@code public static provider()} method.
*
* @see #filterMethods(Method...)
*/
@Substitute //
@SuppressWarnings({"unused"})
List<Method> getDeclaredPublicMethods(String methodName, Class<?>... parameterTypes) {
checkClassFlag(ALL_METHODS_FLAG | ALL_DECLARED_METHODS_FLAG, "getMethods or getDeclaredMethods");

Method[] methods = filterMethods(privateGetDeclaredMethods(/* publicOnly */ true));
Method[] methods = privateGetDeclaredMethods(/* publicOnly */ true);
ReflectionFactory factory = getReflectionFactory();
List<Method> result = new ArrayList<>();
boolean matchedAnyRegistered = false;
for (Method method : methods) {
if (method.getName().equals(methodName) && Arrays.equals(factory.getExecutableSharedParameterTypes(method), parameterTypes)) {
result.add(factory.copyMethod(method));
matchedAnyRegistered = true;
/*
* We've matched a registered method query, but we still need to check it's not a
* negative method or hiding method.
*/
if (checkMethodExists(methodName, parameterTypes, method, /* publicOnly */ true)) {
result.add(factory.copyMethod(method));
}
}
}
if (!matchedAnyRegistered) {
/*
* No matching method was registered. Report a missing registration error if no bulk
* query for all (public or declared) methods was registered for reflection either.
*/
checkMethodExists(methodName, parameterTypes, null, /* publicOnly */ true);
}
return result;
}

Expand Down
58 changes: 48 additions & 10 deletions substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ServiceLoaderFeature.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 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
Expand All @@ -25,6 +25,8 @@
package com.oracle.svm.hosted;

import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.nio.charset.StandardCharsets;
import java.util.Collection;
import java.util.HashSet;
Expand All @@ -33,8 +35,6 @@
import java.util.Set;
import java.util.stream.Collectors;

import jdk.graal.compiler.options.Option;
import jdk.graal.compiler.options.OptionType;
import org.graalvm.nativeimage.hosted.RuntimeReflection;
import org.graalvm.nativeimage.hosted.RuntimeResourceAccess;

Expand All @@ -45,6 +45,9 @@
import com.oracle.svm.core.option.LocatableMultiOptionValue;
import com.oracle.svm.hosted.analysis.Inflation;

import jdk.graal.compiler.options.Option;
import jdk.graal.compiler.options.OptionType;

/**
* Support for {@link ServiceLoader} on Substrate VM.
*
Expand Down Expand Up @@ -179,17 +182,52 @@ void handleServiceClassIsReachable(DuringAnalysisAccess access, Class<?> service
continue;
}

Constructor<?> nullaryConstructor;
/*
* Find either a public static provider() method or a nullary constructor (or both).
* Skip providers that do not comply with requirements.
*
* See ServiceLoader#loadProvider and ServiceLoader#findStaticProviderMethod.
*/
Constructor<?> nullaryConstructor = null;
Method nullaryProviderMethod = null;
try {
nullaryConstructor = providerClass.getDeclaredConstructor();
/* Only look for a provider() method if provider class is in an explicit module. */
if (providerClass.getModule().isNamed() && !providerClass.getModule().getDescriptor().isAutomatic()) {
for (Method method : providerClass.getDeclaredMethods()) {
if (Modifier.isPublic(method.getModifiers()) && Modifier.isStatic(method.getModifiers()) &&
method.getParameterCount() == 0 && method.getName().equals("provider")) {
if (nullaryProviderMethod == null) {
nullaryProviderMethod = method;
} else {
/* There must be at most one public static provider() method. */
nullaryProviderMethod = null;
break;
}
}
}
}

Constructor<?> constructor = providerClass.getDeclaredConstructor();
if (Modifier.isPublic(constructor.getModifiers())) {
nullaryConstructor = constructor;
}
} catch (NoSuchMethodException | SecurityException | LinkageError e) {
/* Skip providers that do not comply with requirements */
nullaryConstructor = null;
// ignore
}
if (nullaryConstructor != null) {
if (nullaryConstructor != null || nullaryProviderMethod != null) {
RuntimeReflection.register(providerClass);
RuntimeReflection.register(nullaryConstructor);
RuntimeReflection.registerAllDeclaredMethods(providerClass);
if (nullaryConstructor != null) {
RuntimeReflection.register(nullaryConstructor);
}
if (nullaryProviderMethod != null) {
RuntimeReflection.register(nullaryProviderMethod);
} else {
/*
* If there's no declared public provider() method, register it as negative
* lookup to avoid throwing a MissingReflectionRegistrationError at run time.
*/
RuntimeReflection.registerMethodLookup(providerClass, "provider");
}
registeredProviders.add(provider);
}
}
Expand Down

0 comments on commit b26cb04

Please sign in to comment.