From bc1f1462e839eb38cb37c86f2e7edb41826082ba Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 8 Oct 2025 11:02:49 -0700 Subject: [PATCH 1/3] api: Remove unused ServiceProviders.load() load() unused since c8a94d105 in 2020. --- .../io/grpc/InternalServiceProviders.java | 11 ------- .../main/java/io/grpc/ServiceProviders.java | 17 ---------- .../java/io/grpc/ServiceProvidersTest.java | 32 ++++++++++++------- 3 files changed, 20 insertions(+), 40 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalServiceProviders.java b/api/src/main/java/io/grpc/InternalServiceProviders.java index c72e01db67a..9207f6f741c 100644 --- a/api/src/main/java/io/grpc/InternalServiceProviders.java +++ b/api/src/main/java/io/grpc/InternalServiceProviders.java @@ -24,17 +24,6 @@ public final class InternalServiceProviders { private InternalServiceProviders() { } - /** - * Accessor for method. - */ - public static T load( - Class klass, - Iterable> hardcoded, - ClassLoader classLoader, - PriorityAccessor priorityAccessor) { - return ServiceProviders.load(klass, hardcoded, classLoader, priorityAccessor); - } - /** * Accessor for method. */ diff --git a/api/src/main/java/io/grpc/ServiceProviders.java b/api/src/main/java/io/grpc/ServiceProviders.java index ac4b27d8783..c7a118ba042 100644 --- a/api/src/main/java/io/grpc/ServiceProviders.java +++ b/api/src/main/java/io/grpc/ServiceProviders.java @@ -29,23 +29,6 @@ private ServiceProviders() { // do not instantiate } - /** - * If this is not Android, returns the highest priority implementation of the class via - * {@link ServiceLoader}. - * If this is Android, returns an instance of the highest priority class in {@code hardcoded}. - */ - public static T load( - Class klass, - Iterable> hardcoded, - ClassLoader cl, - PriorityAccessor priorityAccessor) { - List candidates = loadAll(klass, hardcoded, cl, priorityAccessor); - if (candidates.isEmpty()) { - return null; - } - return candidates.get(0); - } - /** * If this is not Android, returns all available implementations discovered via * {@link ServiceLoader}. diff --git a/api/src/test/java/io/grpc/ServiceProvidersTest.java b/api/src/test/java/io/grpc/ServiceProvidersTest.java index 7d4388a5bb9..63809351c58 100644 --- a/api/src/test/java/io/grpc/ServiceProvidersTest.java +++ b/api/src/test/java/io/grpc/ServiceProvidersTest.java @@ -69,8 +69,7 @@ public void contextClassLoaderProvider() { Thread.currentThread().setContextClassLoader(rcll); assertEquals( Available7Provider.class, - ServiceProviders.load( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); + load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); } finally { Thread.currentThread().setContextClassLoader(ccl); } @@ -85,8 +84,7 @@ public void noProvider() { serviceFile, "io/grpc/ServiceProvidersTestAbstractProvider-doesNotExist.txt"); Thread.currentThread().setContextClassLoader(cl); - assertNull(ServiceProviders.load( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR)); + assertNull(load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR)); } finally { Thread.currentThread().setContextClassLoader(ccl); } @@ -98,8 +96,7 @@ public void multipleProvider() throws Exception { "io/grpc/ServiceProvidersTestAbstractProvider-multipleProvider.txt"); assertSame( Available7Provider.class, - ServiceProviders.load( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); + load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); List providers = ServiceProviders.loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); @@ -116,8 +113,7 @@ public void unavailableProvider() { "io/grpc/ServiceProvidersTestAbstractProvider-unavailableProvider.txt"); assertEquals( Available7Provider.class, - ServiceProviders.load( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); + load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); } @Test @@ -125,7 +121,7 @@ public void unknownClassProvider() { ClassLoader cl = new ReplacingClassLoader(getClass().getClassLoader(), serviceFile, "io/grpc/ServiceProvidersTestAbstractProvider-unknownClassProvider.txt"); try { - ServiceProviders.load( + ServiceProviders.loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Exception expected"); } catch (ServiceConfigurationError e) { @@ -140,7 +136,7 @@ public void exceptionSurfacedToCaller_failAtInit() { try { // Even though there is a working provider, if any providers fail then we should fail // completely to avoid returning something unexpected. - ServiceProviders.load( + ServiceProviders.loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Expected exception"); } catch (ServiceConfigurationError expected) { @@ -154,7 +150,7 @@ public void exceptionSurfacedToCaller_failAtPriority() { "io/grpc/ServiceProvidersTestAbstractProvider-failAtPriorityProvider.txt"); try { // The exception should be surfaced to the caller - ServiceProviders.load( + ServiceProviders.loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Expected exception"); } catch (FailAtPriorityProvider.PriorityException expected) { @@ -168,7 +164,7 @@ public void exceptionSurfacedToCaller_failAtAvailable() { "io/grpc/ServiceProvidersTestAbstractProvider-failAtAvailableProvider.txt"); try { // The exception should be surfaced to the caller - ServiceProviders.load( + ServiceProviders.loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Expected exception"); } catch (FailAtAvailableProvider.AvailableException expected) { @@ -244,6 +240,18 @@ class RandomClass {} assertFalse(candidates.iterator().hasNext()); } + private static T load( + Class klass, + Iterable> hardcoded, + ClassLoader cl, + PriorityAccessor priorityAccessor) { + List candidates = ServiceProviders.loadAll(klass, hardcoded, cl, priorityAccessor); + if (candidates.isEmpty()) { + return null; + } + return candidates.get(0); + } + private static class BaseProvider extends ServiceProvidersTestAbstractProvider { private final boolean isAvailable; private final int priority; From 14c4a25e17b528ee7a49bde930998f435afed5d0 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 15 Jan 2026 14:10:12 -0800 Subject: [PATCH 2/3] Trigger R8's ServiceLoader optimization This simplifies R8 Full Mode's configuration when paired with R8 optimizations (which is made more difficult to avoid in AGP 9), as when the optimization is triggered Full Mode will automatically keep the constructor for the relevant classes. android-interop-test doesn't currently enable R8 optimizations, so it doesn't actually demonstrate the benefit, but I have manually confirmed that enabling proguard-android-optimize.txt does cause the ServiceLoader optimization in android-interop-test. Note that full mode is a separate configuration and not necessary to get the ServiceLoader optimization. --- .../io/grpc/InternalServiceProviders.java | 20 +++++++++- .../java/io/grpc/LoadBalancerRegistry.java | 5 ++- .../java/io/grpc/ManagedChannelRegistry.java | 5 ++- .../java/io/grpc/NameResolverRegistry.java | 5 ++- api/src/main/java/io/grpc/ServerRegistry.java | 4 +- .../main/java/io/grpc/ServiceProviders.java | 40 ++++++++++++++----- .../java/io/grpc/ServiceProvidersTest.java | 29 +++++++++----- .../io/grpc/xds/XdsCredentialsRegistry.java | 5 ++- 8 files changed, 87 insertions(+), 26 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalServiceProviders.java b/api/src/main/java/io/grpc/InternalServiceProviders.java index 9207f6f741c..93858fe1408 100644 --- a/api/src/main/java/io/grpc/InternalServiceProviders.java +++ b/api/src/main/java/io/grpc/InternalServiceProviders.java @@ -17,7 +17,9 @@ package io.grpc; import com.google.common.annotations.VisibleForTesting; +import java.util.Iterator; import java.util.List; +import java.util.ServiceLoader; @Internal public final class InternalServiceProviders { @@ -27,12 +29,28 @@ private InternalServiceProviders() { /** * Accessor for method. */ + @Deprecated public static List loadAll( Class klass, Iterable> hardCodedClasses, ClassLoader classLoader, PriorityAccessor priorityAccessor) { - return ServiceProviders.loadAll(klass, hardCodedClasses, classLoader, priorityAccessor); + return loadAll( + klass, + ServiceLoader.load(klass, classLoader).iterator(), + hardCodedClasses, + priorityAccessor); + } + + /** + * Accessor for method. + */ + public static List loadAll( + Class klass, + Iterator serviceLoader, + Iterable> hardCodedClasses, + PriorityAccessor priorityAccessor) { + return ServiceProviders.loadAll(klass, serviceLoader, hardCodedClasses, priorityAccessor); } /** diff --git a/api/src/main/java/io/grpc/LoadBalancerRegistry.java b/api/src/main/java/io/grpc/LoadBalancerRegistry.java index f6b69f978b8..14e606fe8d2 100644 --- a/api/src/main/java/io/grpc/LoadBalancerRegistry.java +++ b/api/src/main/java/io/grpc/LoadBalancerRegistry.java @@ -26,6 +26,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.ServiceLoader; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -101,8 +102,10 @@ public static synchronized LoadBalancerRegistry getDefaultRegistry() { if (instance == null) { List providerList = ServiceProviders.loadAll( LoadBalancerProvider.class, + ServiceLoader + .load(LoadBalancerProvider.class, LoadBalancerProvider.class.getClassLoader()) + .iterator(), HARDCODED_CLASSES, - LoadBalancerProvider.class.getClassLoader(), new LoadBalancerPriorityAccessor()); instance = new LoadBalancerRegistry(); for (LoadBalancerProvider provider : providerList) { diff --git a/api/src/main/java/io/grpc/ManagedChannelRegistry.java b/api/src/main/java/io/grpc/ManagedChannelRegistry.java index aed5eca9abf..6060a65c144 100644 --- a/api/src/main/java/io/grpc/ManagedChannelRegistry.java +++ b/api/src/main/java/io/grpc/ManagedChannelRegistry.java @@ -29,6 +29,7 @@ import java.util.Comparator; import java.util.LinkedHashSet; import java.util.List; +import java.util.ServiceLoader; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.concurrent.ThreadSafe; @@ -100,8 +101,10 @@ public static synchronized ManagedChannelRegistry getDefaultRegistry() { if (instance == null) { List providerList = ServiceProviders.loadAll( ManagedChannelProvider.class, + ServiceLoader + .load(ManagedChannelProvider.class, ManagedChannelProvider.class.getClassLoader()) + .iterator(), getHardCodedClasses(), - ManagedChannelProvider.class.getClassLoader(), new ManagedChannelPriorityAccessor()); instance = new ManagedChannelRegistry(); for (ManagedChannelProvider provider : providerList) { diff --git a/api/src/main/java/io/grpc/NameResolverRegistry.java b/api/src/main/java/io/grpc/NameResolverRegistry.java index 26eb5552b9b..4b536a8c8ea 100644 --- a/api/src/main/java/io/grpc/NameResolverRegistry.java +++ b/api/src/main/java/io/grpc/NameResolverRegistry.java @@ -29,6 +29,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.ServiceLoader; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -125,8 +126,10 @@ public static synchronized NameResolverRegistry getDefaultRegistry() { if (instance == null) { List providerList = ServiceProviders.loadAll( NameResolverProvider.class, + ServiceLoader + .load(NameResolverProvider.class, NameResolverProvider.class.getClassLoader()) + .iterator(), getHardCodedClasses(), - NameResolverProvider.class.getClassLoader(), new NameResolverPriorityAccessor()); if (providerList.isEmpty()) { logger.warning("No NameResolverProviders found via ServiceLoader, including for DNS. This " diff --git a/api/src/main/java/io/grpc/ServerRegistry.java b/api/src/main/java/io/grpc/ServerRegistry.java index 5b9c8c558e7..4dde385f46b 100644 --- a/api/src/main/java/io/grpc/ServerRegistry.java +++ b/api/src/main/java/io/grpc/ServerRegistry.java @@ -24,6 +24,7 @@ import java.util.Comparator; import java.util.LinkedHashSet; import java.util.List; +import java.util.ServiceLoader; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.concurrent.ThreadSafe; @@ -93,8 +94,9 @@ public static synchronized ServerRegistry getDefaultRegistry() { if (instance == null) { List providerList = ServiceProviders.loadAll( ServerProvider.class, + ServiceLoader.load(ServerProvider.class, ServerProvider.class.getClassLoader()) + .iterator(), getHardCodedClasses(), - ServerProvider.class.getClassLoader(), new ServerPriorityAccessor()); instance = new ServerRegistry(); for (ServerProvider provider : providerList) { diff --git a/api/src/main/java/io/grpc/ServiceProviders.java b/api/src/main/java/io/grpc/ServiceProviders.java index c7a118ba042..4634368432a 100644 --- a/api/src/main/java/io/grpc/ServiceProviders.java +++ b/api/src/main/java/io/grpc/ServiceProviders.java @@ -20,7 +20,9 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.Iterator; import java.util.List; +import java.util.ListIterator; import java.util.ServiceConfigurationError; import java.util.ServiceLoader; @@ -34,20 +36,39 @@ private ServiceProviders() { * {@link ServiceLoader}. * If this is Android, returns all available implementations in {@code hardcoded}. * The list is sorted in descending priority order. + * + *

{@code serviceLoader} should be created with {@code ServiceLoader.load(MyClass.class, + * MyClass.class.getClassLoader()).iterator()} in order to be detected by R8 so that R8 full mode + * will keep the constructors for the provider classes. */ public static List loadAll( Class klass, + Iterator serviceLoader, Iterable> hardcoded, - ClassLoader cl, final PriorityAccessor priorityAccessor) { - Iterable candidates; - if (isAndroid(cl)) { - candidates = getCandidatesViaHardCoded(klass, hardcoded); + Iterator candidates; + if (serviceLoader instanceof ListIterator) { + // A rewriting tool has replaced the ServiceLoader with a List of some sort (R8 uses + // ArrayList, AppReduce uses singletonList). We prefer to use such iterators on Android as + // they won't need reflection like the hard-coded list does. In addition, the provider + // instances will have already been created, so it seems we should use them. + // + // R8: https://r8.googlesource.com/r8/+/490bc53d9310d4cc2a5084c05df4aadaec8c885d/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java + // AppReduce: service_loader_pass.cc + candidates = serviceLoader; + } else if (isAndroid(klass.getClassLoader())) { + // Avoid getResource() on Android, which must read from a zip which uses a lot of memory + candidates = getCandidatesViaHardCoded(klass, hardcoded).iterator(); + } else if (!serviceLoader.hasNext()) { + // Attempt to load using the context class loader and ServiceLoader. + // This allows frameworks like http://aries.apache.org/modules/spi-fly.html to plug in. + candidates = ServiceLoader.load(klass).iterator(); } else { - candidates = getCandidatesViaServiceLoader(klass, cl); + candidates = serviceLoader; } List list = new ArrayList<>(); - for (T current: candidates) { + while (candidates.hasNext()) { + T current = candidates.next(); if (!priorityAccessor.isAvailable(current)) { continue; } @@ -84,15 +105,14 @@ static boolean isAndroid(ClassLoader cl) { } /** - * Loads service providers for the {@code klass} service using {@link ServiceLoader}. + * For testing only: Loads service providers for the {@code klass} service using {@link + * ServiceLoader}. Does not support spi-fly and related tricks. */ @VisibleForTesting public static Iterable getCandidatesViaServiceLoader(Class klass, ClassLoader cl) { Iterable i = ServiceLoader.load(klass, cl); - // Attempt to load using the context class loader and ServiceLoader. - // This allows frameworks like http://aries.apache.org/modules/spi-fly.html to plug in. if (!i.iterator().hasNext()) { - i = ServiceLoader.load(klass); + return null; } return i; } diff --git a/api/src/test/java/io/grpc/ServiceProvidersTest.java b/api/src/test/java/io/grpc/ServiceProvidersTest.java index 63809351c58..33ebdd0db25 100644 --- a/api/src/test/java/io/grpc/ServiceProvidersTest.java +++ b/api/src/test/java/io/grpc/ServiceProvidersTest.java @@ -29,6 +29,7 @@ import java.util.Iterator; import java.util.List; import java.util.ServiceConfigurationError; +import java.util.ServiceLoader; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -98,7 +99,7 @@ public void multipleProvider() throws Exception { Available7Provider.class, load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); - List providers = ServiceProviders.loadAll( + List providers = loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); assertEquals(3, providers.size()); assertEquals(Available7Provider.class, providers.get(0).getClass()); @@ -121,8 +122,7 @@ public void unknownClassProvider() { ClassLoader cl = new ReplacingClassLoader(getClass().getClassLoader(), serviceFile, "io/grpc/ServiceProvidersTestAbstractProvider-unknownClassProvider.txt"); try { - ServiceProviders.loadAll( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); + loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Exception expected"); } catch (ServiceConfigurationError e) { // noop @@ -136,8 +136,7 @@ public void exceptionSurfacedToCaller_failAtInit() { try { // Even though there is a working provider, if any providers fail then we should fail // completely to avoid returning something unexpected. - ServiceProviders.loadAll( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); + loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Expected exception"); } catch (ServiceConfigurationError expected) { // noop @@ -150,8 +149,7 @@ public void exceptionSurfacedToCaller_failAtPriority() { "io/grpc/ServiceProvidersTestAbstractProvider-failAtPriorityProvider.txt"); try { // The exception should be surfaced to the caller - ServiceProviders.loadAll( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); + loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Expected exception"); } catch (FailAtPriorityProvider.PriorityException expected) { // noop @@ -164,8 +162,7 @@ public void exceptionSurfacedToCaller_failAtAvailable() { "io/grpc/ServiceProvidersTestAbstractProvider-failAtAvailableProvider.txt"); try { // The exception should be surfaced to the caller - ServiceProviders.loadAll( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); + loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Expected exception"); } catch (FailAtAvailableProvider.AvailableException expected) { // noop @@ -245,13 +242,25 @@ private static T load( Iterable> hardcoded, ClassLoader cl, PriorityAccessor priorityAccessor) { - List candidates = ServiceProviders.loadAll(klass, hardcoded, cl, priorityAccessor); + List candidates = loadAll(klass, hardcoded, cl, priorityAccessor); if (candidates.isEmpty()) { return null; } return candidates.get(0); } + private static List loadAll( + Class klass, + Iterable> hardcoded, + ClassLoader classLoader, + PriorityAccessor priorityAccessor) { + return ServiceProviders.loadAll( + klass, + ServiceLoader.load(klass, classLoader).iterator(), + hardcoded, + priorityAccessor); + } + private static class BaseProvider extends ServiceProvidersTestAbstractProvider { private final boolean isAvailable; private final int priority; diff --git a/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java b/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java index 9dfefaf1a65..051ff04ad25 100644 --- a/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java +++ b/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java @@ -29,6 +29,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.ServiceLoader; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -109,8 +110,10 @@ public static synchronized XdsCredentialsRegistry getDefaultRegistry() { if (instance == null) { List providerList = InternalServiceProviders.loadAll( XdsCredentialsProvider.class, + ServiceLoader + .load(XdsCredentialsProvider.class, XdsCredentialsProvider.class.getClassLoader()) + .iterator(), getHardCodedClasses(), - XdsCredentialsProvider.class.getClassLoader(), new XdsCredentialsProviderPriorityAccessor()); if (providerList.isEmpty()) { logger.warning("No XdsCredsRegistry found via ServiceLoader, including for GoogleDefault, " From 77fd613fbbeb468bdb270e60b2400bbf07f26176 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Fri, 16 Jan 2026 13:49:37 -0800 Subject: [PATCH 3/3] Only create hard-coded when it is being used --- .../io/grpc/InternalServiceProviders.java | 10 ++-- .../java/io/grpc/LoadBalancerRegistry.java | 3 +- .../java/io/grpc/ManagedChannelRegistry.java | 2 +- .../java/io/grpc/NameResolverRegistry.java | 2 +- api/src/main/java/io/grpc/ServerRegistry.java | 2 +- .../main/java/io/grpc/ServiceProviders.java | 5 +- .../java/io/grpc/ServiceProvidersTest.java | 46 +++++++++++++------ .../io/grpc/xds/XdsCredentialsRegistry.java | 2 +- 8 files changed, 47 insertions(+), 25 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalServiceProviders.java b/api/src/main/java/io/grpc/InternalServiceProviders.java index 93858fe1408..debc786a82a 100644 --- a/api/src/main/java/io/grpc/InternalServiceProviders.java +++ b/api/src/main/java/io/grpc/InternalServiceProviders.java @@ -38,7 +38,7 @@ public static List loadAll( return loadAll( klass, ServiceLoader.load(klass, classLoader).iterator(), - hardCodedClasses, + () -> hardCodedClasses, priorityAccessor); } @@ -48,9 +48,9 @@ public static List loadAll( public static List loadAll( Class klass, Iterator serviceLoader, - Iterable> hardCodedClasses, + Supplier>> hardCodedClasses, PriorityAccessor priorityAccessor) { - return ServiceProviders.loadAll(klass, serviceLoader, hardCodedClasses, priorityAccessor); + return ServiceProviders.loadAll(klass, serviceLoader, hardCodedClasses::get, priorityAccessor); } /** @@ -78,4 +78,8 @@ public static boolean isAndroid(ClassLoader cl) { } public interface PriorityAccessor extends ServiceProviders.PriorityAccessor {} + + public interface Supplier { + T get(); + } } diff --git a/api/src/main/java/io/grpc/LoadBalancerRegistry.java b/api/src/main/java/io/grpc/LoadBalancerRegistry.java index 14e606fe8d2..a8fbc102f5f 100644 --- a/api/src/main/java/io/grpc/LoadBalancerRegistry.java +++ b/api/src/main/java/io/grpc/LoadBalancerRegistry.java @@ -43,7 +43,6 @@ public final class LoadBalancerRegistry { private static final Logger logger = Logger.getLogger(LoadBalancerRegistry.class.getName()); private static LoadBalancerRegistry instance; - private static final Iterable> HARDCODED_CLASSES = getHardCodedClasses(); private final LinkedHashSet allProviders = new LinkedHashSet<>(); @@ -105,7 +104,7 @@ public static synchronized LoadBalancerRegistry getDefaultRegistry() { ServiceLoader .load(LoadBalancerProvider.class, LoadBalancerProvider.class.getClassLoader()) .iterator(), - HARDCODED_CLASSES, + LoadBalancerRegistry::getHardCodedClasses, new LoadBalancerPriorityAccessor()); instance = new LoadBalancerRegistry(); for (LoadBalancerProvider provider : providerList) { diff --git a/api/src/main/java/io/grpc/ManagedChannelRegistry.java b/api/src/main/java/io/grpc/ManagedChannelRegistry.java index 6060a65c144..9b782dcf48e 100644 --- a/api/src/main/java/io/grpc/ManagedChannelRegistry.java +++ b/api/src/main/java/io/grpc/ManagedChannelRegistry.java @@ -104,7 +104,7 @@ public static synchronized ManagedChannelRegistry getDefaultRegistry() { ServiceLoader .load(ManagedChannelProvider.class, ManagedChannelProvider.class.getClassLoader()) .iterator(), - getHardCodedClasses(), + ManagedChannelRegistry::getHardCodedClasses, new ManagedChannelPriorityAccessor()); instance = new ManagedChannelRegistry(); for (ManagedChannelProvider provider : providerList) { diff --git a/api/src/main/java/io/grpc/NameResolverRegistry.java b/api/src/main/java/io/grpc/NameResolverRegistry.java index 4b536a8c8ea..c7e5cf30714 100644 --- a/api/src/main/java/io/grpc/NameResolverRegistry.java +++ b/api/src/main/java/io/grpc/NameResolverRegistry.java @@ -129,7 +129,7 @@ public static synchronized NameResolverRegistry getDefaultRegistry() { ServiceLoader .load(NameResolverProvider.class, NameResolverProvider.class.getClassLoader()) .iterator(), - getHardCodedClasses(), + NameResolverRegistry::getHardCodedClasses, new NameResolverPriorityAccessor()); if (providerList.isEmpty()) { logger.warning("No NameResolverProviders found via ServiceLoader, including for DNS. This " diff --git a/api/src/main/java/io/grpc/ServerRegistry.java b/api/src/main/java/io/grpc/ServerRegistry.java index 4dde385f46b..1ec7030b82b 100644 --- a/api/src/main/java/io/grpc/ServerRegistry.java +++ b/api/src/main/java/io/grpc/ServerRegistry.java @@ -96,7 +96,7 @@ public static synchronized ServerRegistry getDefaultRegistry() { ServerProvider.class, ServiceLoader.load(ServerProvider.class, ServerProvider.class.getClassLoader()) .iterator(), - getHardCodedClasses(), + ServerRegistry::getHardCodedClasses, new ServerPriorityAccessor()); instance = new ServerRegistry(); for (ServerProvider provider : providerList) { diff --git a/api/src/main/java/io/grpc/ServiceProviders.java b/api/src/main/java/io/grpc/ServiceProviders.java index 4634368432a..861688be9fb 100644 --- a/api/src/main/java/io/grpc/ServiceProviders.java +++ b/api/src/main/java/io/grpc/ServiceProviders.java @@ -17,6 +17,7 @@ package io.grpc; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Supplier; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -44,7 +45,7 @@ private ServiceProviders() { public static List loadAll( Class klass, Iterator serviceLoader, - Iterable> hardcoded, + Supplier>> hardcoded, final PriorityAccessor priorityAccessor) { Iterator candidates; if (serviceLoader instanceof ListIterator) { @@ -58,7 +59,7 @@ public static List loadAll( candidates = serviceLoader; } else if (isAndroid(klass.getClassLoader())) { // Avoid getResource() on Android, which must read from a zip which uses a lot of memory - candidates = getCandidatesViaHardCoded(klass, hardcoded).iterator(); + candidates = getCandidatesViaHardCoded(klass, hardcoded.get()).iterator(); } else if (!serviceLoader.hasNext()) { // Attempt to load using the context class loader and ServiceLoader. // This allows frameworks like http://aries.apache.org/modules/spi-fly.html to plug in. diff --git a/api/src/test/java/io/grpc/ServiceProvidersTest.java b/api/src/test/java/io/grpc/ServiceProvidersTest.java index 33ebdd0db25..f971ed42646 100644 --- a/api/src/test/java/io/grpc/ServiceProvidersTest.java +++ b/api/src/test/java/io/grpc/ServiceProvidersTest.java @@ -16,6 +16,7 @@ package io.grpc; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -23,6 +24,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import io.grpc.InternalServiceProviders.PriorityAccessor; import java.util.Collections; @@ -30,6 +32,7 @@ import java.util.List; import java.util.ServiceConfigurationError; import java.util.ServiceLoader; +import org.junit.After; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -37,7 +40,6 @@ /** Unit tests for {@link ServiceProviders}. */ @RunWith(JUnit4.class) public class ServiceProvidersTest { - private static final List> NO_HARDCODED = Collections.emptyList(); private static final PriorityAccessor ACCESSOR = new PriorityAccessor() { @Override @@ -52,6 +54,19 @@ public int getPriority(ServiceProvidersTestAbstractProvider provider) { }; private final String serviceFile = "META-INF/services/io.grpc.ServiceProvidersTestAbstractProvider"; + private boolean failingHardCodedAccessed; + private final Supplier>> failingHardCoded = new Supplier>>() { + @Override + public Iterable> get() { + failingHardCodedAccessed = true; + throw new AssertionError(); + } + }; + + @After + public void tearDown() { + assertThat(failingHardCodedAccessed).isFalse(); + } @Test public void contextClassLoaderProvider() { @@ -70,7 +85,8 @@ public void contextClassLoaderProvider() { Thread.currentThread().setContextClassLoader(rcll); assertEquals( Available7Provider.class, - load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); + load(ServiceProvidersTestAbstractProvider.class, failingHardCoded, cl, ACCESSOR) + .getClass()); } finally { Thread.currentThread().setContextClassLoader(ccl); } @@ -85,7 +101,7 @@ public void noProvider() { serviceFile, "io/grpc/ServiceProvidersTestAbstractProvider-doesNotExist.txt"); Thread.currentThread().setContextClassLoader(cl); - assertNull(load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR)); + assertNull(load(ServiceProvidersTestAbstractProvider.class, failingHardCoded, cl, ACCESSOR)); } finally { Thread.currentThread().setContextClassLoader(ccl); } @@ -97,10 +113,11 @@ public void multipleProvider() throws Exception { "io/grpc/ServiceProvidersTestAbstractProvider-multipleProvider.txt"); assertSame( Available7Provider.class, - load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); + load(ServiceProvidersTestAbstractProvider.class, failingHardCoded, cl, ACCESSOR) + .getClass()); List providers = loadAll( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); + ServiceProvidersTestAbstractProvider.class, failingHardCoded, cl, ACCESSOR); assertEquals(3, providers.size()); assertEquals(Available7Provider.class, providers.get(0).getClass()); assertEquals(Available5Provider.class, providers.get(1).getClass()); @@ -114,7 +131,8 @@ public void unavailableProvider() { "io/grpc/ServiceProvidersTestAbstractProvider-unavailableProvider.txt"); assertEquals( Available7Provider.class, - load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); + load(ServiceProvidersTestAbstractProvider.class, failingHardCoded, cl, ACCESSOR) + .getClass()); } @Test @@ -122,7 +140,7 @@ public void unknownClassProvider() { ClassLoader cl = new ReplacingClassLoader(getClass().getClassLoader(), serviceFile, "io/grpc/ServiceProvidersTestAbstractProvider-unknownClassProvider.txt"); try { - loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); + loadAll(ServiceProvidersTestAbstractProvider.class, failingHardCoded, cl, ACCESSOR); fail("Exception expected"); } catch (ServiceConfigurationError e) { // noop @@ -136,7 +154,7 @@ public void exceptionSurfacedToCaller_failAtInit() { try { // Even though there is a working provider, if any providers fail then we should fail // completely to avoid returning something unexpected. - loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); + loadAll(ServiceProvidersTestAbstractProvider.class, failingHardCoded, cl, ACCESSOR); fail("Expected exception"); } catch (ServiceConfigurationError expected) { // noop @@ -149,7 +167,7 @@ public void exceptionSurfacedToCaller_failAtPriority() { "io/grpc/ServiceProvidersTestAbstractProvider-failAtPriorityProvider.txt"); try { // The exception should be surfaced to the caller - loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); + loadAll(ServiceProvidersTestAbstractProvider.class, failingHardCoded, cl, ACCESSOR); fail("Expected exception"); } catch (FailAtPriorityProvider.PriorityException expected) { // noop @@ -162,7 +180,7 @@ public void exceptionSurfacedToCaller_failAtAvailable() { "io/grpc/ServiceProvidersTestAbstractProvider-failAtAvailableProvider.txt"); try { // The exception should be surfaced to the caller - loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); + loadAll(ServiceProvidersTestAbstractProvider.class, failingHardCoded, cl, ACCESSOR); fail("Expected exception"); } catch (FailAtAvailableProvider.AvailableException expected) { // noop @@ -239,10 +257,10 @@ class RandomClass {} private static T load( Class klass, - Iterable> hardcoded, + Supplier>> hardCoded, ClassLoader cl, PriorityAccessor priorityAccessor) { - List candidates = loadAll(klass, hardcoded, cl, priorityAccessor); + List candidates = loadAll(klass, hardCoded, cl, priorityAccessor); if (candidates.isEmpty()) { return null; } @@ -251,13 +269,13 @@ private static T load( private static List loadAll( Class klass, - Iterable> hardcoded, + Supplier>> hardCoded, ClassLoader classLoader, PriorityAccessor priorityAccessor) { return ServiceProviders.loadAll( klass, ServiceLoader.load(klass, classLoader).iterator(), - hardcoded, + hardCoded, priorityAccessor); } diff --git a/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java b/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java index 051ff04ad25..9dd77a400cd 100644 --- a/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java +++ b/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java @@ -113,7 +113,7 @@ public static synchronized XdsCredentialsRegistry getDefaultRegistry() { ServiceLoader .load(XdsCredentialsProvider.class, XdsCredentialsProvider.class.getClassLoader()) .iterator(), - getHardCodedClasses(), + XdsCredentialsRegistry::getHardCodedClasses, new XdsCredentialsProviderPriorityAccessor()); if (providerList.isEmpty()) { logger.warning("No XdsCredsRegistry found via ServiceLoader, including for GoogleDefault, "