core: Stop using context ClassLoader in Providers

ejona86 · ejona86 · commit c637b42b7157 · 2017-07-06T11:02:31.000-07:00
currentThread().getContextClassLoader() was originally used to match ServiceLoader's default. ServiceLoader is mostly used with SPIs that exist in the bootclasspath and so the class's ClassLoader would not work. But in gRPC we have a library in the application's ClassLoader. Context ClassLoader has been causing problems in any case more than one application ClassLoader exists, like in Servlet containers, Android, and plugins. To my knowledge, in every case the context class loader is different from the provider's class loader, the context class loader has caused breakage, not success. In addition, since we use static fields for storing the provider results, using anything but the provider's class loader is asking to have results that vary simply depending on which thread called gRPC first. Fixes grpc#2375
diff --git a/core/src/main/java/io/grpc/ManagedChannelProvider.java b/core/src/main/java/io/grpc/ManagedChannelProvider.java
@@ -34,7 +34,7 @@
 @Internal
 public abstract class ManagedChannelProvider {
   private static final ManagedChannelProvider provider
-      = load(getCorrectClassLoader());
+      = load(ManagedChannelProvider.class.getClassLoader());
 
   @VisibleForTesting
   static ManagedChannelProvider load(ClassLoader classLoader) {
@@ -118,17 +118,6 @@ public static ManagedChannelProvider provider() {
     return provider;
   }
 
-  private static ClassLoader getCorrectClassLoader() {
-    if (isAndroid()) {
-      // When android:sharedUserId or android:process is used, Android will setup a dummy
-      // ClassLoader for the thread context (http://stackoverflow.com/questions/13407006),
-      // instead of letting users to manually set context class loader, we choose the
-      // correct class loader here.
-      return ManagedChannelProvider.class.getClassLoader();
-    }
-    return Thread.currentThread().getContextClassLoader();
-  }
-
   /**
    * Returns whether current platform is Android.
    */
diff --git a/core/src/main/java/io/grpc/NameResolverProvider.java b/core/src/main/java/io/grpc/NameResolverProvider.java
@@ -43,7 +43,7 @@ public abstract class NameResolverProvider extends NameResolver.Factory {
       NameResolver.Factory.PARAMS_DEFAULT_PORT;
 
   private static final List<NameResolverProvider> providers
-      = load(getCorrectClassLoader());
+      = load(NameResolverProvider.class.getClassLoader());
   private static final NameResolver.Factory factory = new NameResolverFactory(providers);
 
   @VisibleForTesting
@@ -120,17 +120,6 @@ static NameResolver.Factory asFactory(List<NameResolverProvider> providers) {
     return new NameResolverFactory(providers);
   }
 
-  private static ClassLoader getCorrectClassLoader() {
-    if (isAndroid()) {
-      // When android:sharedUserId or android:process is used, Android will setup a dummy
-      // ClassLoader for the thread context (http://stackoverflow.com/questions/13407006),
-      // instead of letting users to manually set context class loader, we choose the
-      // correct class loader here.
-      return NameResolverProvider.class.getClassLoader();
-    }
-    return Thread.currentThread().getContextClassLoader();
-  }
-
   private static boolean isAndroid() {
     try {
       Class.forName("android.app.Application", /*initialize=*/ false, null);
diff --git a/core/src/main/java/io/grpc/ServerProvider.java b/core/src/main/java/io/grpc/ServerProvider.java
@@ -30,7 +30,7 @@
 @Internal
 public abstract class ServerProvider {
   private static final ServerProvider provider =
-      load(Thread.currentThread().getContextClassLoader());
+      load(ServerProvider.class.getClassLoader());
 
   @VisibleForTesting
   static final ServerProvider load(ClassLoader cl) {
