HBASE-6308. Coprocessors should be loaded in a custom ClassLoader (James Baldassari)

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1372558 13f79535-47bb-0310-9956-ffa450edef68

hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorClassLoader.java

@@ -0,0 +1,199 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.coprocessor;
+
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.List;
+import java.util.regex.Pattern;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+/**
+ * ClassLoader used to load Coprocessor instances.
+ * 
+ * This ClassLoader always tries to load classes from the Coprocessor jar first 
+ * before delegating to the parent ClassLoader, thus avoiding dependency 
+ * conflicts between HBase's classpath and classes in the coprocessor's jar.  
+ * Certain classes are exempt from being loaded by this ClassLoader because it 
+ * would prevent them from being cast to the equivalent classes in the region 
+ * server.  For example, the Coprocessor interface needs to be loaded by the 
+ * region server's ClassLoader to prevent a ClassCastException when casting the 
+ * coprocessor implementation.
+ * 
+ * This ClassLoader also handles resource loading.  In most cases this 
+ * ClassLoader will attempt to load resources from the coprocessor jar first 
+ * before delegating to the parent.  However, like in class loading, 
+ * some resources need to be handled differently.  For all of the Hadoop 
+ * default configurations (e.g. hbase-default.xml) we will check the parent 
+ * ClassLoader first to prevent issues such as failing the HBase default 
+ * configuration version check.
+ */
+public class CoprocessorClassLoader extends URLClassLoader {
+  private static final Log LOG = 
+      LogFactory.getLog(CoprocessorClassLoader.class);
+  
+  /**
+   * If the class being loaded starts with any of these strings, we will skip
+   * trying to load it from the coprocessor jar and instead delegate 
+   * directly to the parent ClassLoader.
+   */
+  private static final String[] CLASS_PREFIX_EXEMPTIONS = new String[] {
+    // Java standard library:
+    "com.sun.",
+    "launcher.",
+    "java.",
+    "javax.",
+    "org.ietf",
+    "org.omg",
+    "org.w3c",
+    "org.xml",
+    "sunw.",
+    // Hadoop/HBase:
+    "org.apache.hadoop",
+  };
+  
+  /**
+   * If the resource being loaded matches any of these patterns, we will first 
+   * attempt to load the resource with the parent ClassLoader.  Only if the 
+   * resource is not found by the parent do we attempt to load it from the 
+   * coprocessor jar.
+   */
+  private static final Pattern[] RESOURCE_LOAD_PARENT_FIRST_PATTERNS = 
+      new Pattern[] {
+    Pattern.compile("^[^-]+-default\\.xml$")
+  };
+  
+  /**
+   * Creates a CoprocessorClassLoader that loads classes from the given paths.
+   * @param paths paths from which to load classes.
+   * @param parent the parent ClassLoader to set.
+   */
+  public CoprocessorClassLoader(List<URL> paths, ClassLoader parent) {
+    super(paths.toArray(new URL[]{}), parent);
+  }
+  
+  @Override
+  synchronized public Class<?> loadClass(String name) 
+      throws ClassNotFoundException {
+    // Delegate to the parent immediately if this class is exempt
+    if (isClassExempt(name)) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Skipping exempt class " + name + 
+            " - delegating directly to parent");
+      }
+      return super.loadClass(name);
+    }
+    
+    // Check whether the class has already been loaded:
+    Class<?> clasz = findLoadedClass(name);
+    if (clasz != null) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Class " + name + " already loaded");
+      }
+    }
+    else {
+      try {
+        // Try to find this class using the URLs passed to this ClassLoader, 
+        // which includes the coprocessor jar
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Finding class: " + name);
+        }
+        clasz = findClass(name);
+      } catch (ClassNotFoundException e) {
+        // Class not found using this ClassLoader, so delegate to parent
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Class " + name + " not found - delegating to parent");
+        }
+        try {
+          clasz = super.loadClass(name);
+        } catch (ClassNotFoundException e2) {
+          // Class not found in this ClassLoader or in the parent ClassLoader
+          // Log some debug output before rethrowing ClassNotFoundException
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("Class " + name + " not found in parent loader");
+          }
+          throw e2;
+        }
+      }
+    }
+    
+    return clasz;
+  }
+  
+  @Override
+  synchronized public URL getResource(String name) {
+    URL resource = null;
+    boolean parentLoaded = false;
+    
+    // Delegate to the parent first if necessary
+    if (loadResourceUsingParentFirst(name)) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Checking parent first for resource " + name);
+      }
+      resource = super.getResource(name);
+      parentLoaded = true;
+    }
+    
+    if (resource == null) {
+      // Try to find the resource in the coprocessor jar
+      resource = findResource(name);
+      if ((resource == null) && !parentLoaded) {
+        // Not found in the coprocessor jar and we haven't attempted to load 
+        // the resource in the parent yet; fall back to the parent
+        resource = super.getResource(name);
+      }
+    }
+
+    return resource;
+  }
+  
+  /**
+   * Determines whether the given class should be exempt from being loaded 
+   * by this ClassLoader.
+   * @param name the name of the class to test.
+   * @return true if the class should *not* be loaded by this ClassLoader; 
+   * false otherwise.
+   */
+  protected boolean isClassExempt(String name) {
+    for (String exemptPrefix : CLASS_PREFIX_EXEMPTIONS) {
+      if (name.startsWith(exemptPrefix)) {
+        return true;
+      }
+    }
+    return false;
+  }
+  
+  /**
+   * Determines whether we should attempt to load the given resource using the  
+   * parent first before attempting to load the resource using this ClassLoader.
+   * @param name the name of the resource to test.
+   * @return true if we should attempt to load the resource using the parent 
+   * first; false if we should attempt to load the resource using this 
+   * ClassLoader first.
+   */
+  protected boolean loadResourceUsingParentFirst(String name) {
+    for (Pattern resourcePattern : RESOURCE_LOAD_PARENT_FIRST_PATTERNS) {
+      if (resourcePattern.matcher(name).matches()) {
+        return true;
+      }
+    }
+    return false;
+  }
+}

hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java

@@ -45,7 +45,6 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.net.URL;
-import java.net.URLClassLoader;
 import java.util.*;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
@@ -161,6 +160,8 @@ public abstract class CoprocessorHost<E extends CoprocessorEnvironment> {
   public E load(Path path, String className, int priority,
       Configuration conf) throws IOException {
     Class<?> implClass = null;
+    LOG.debug("Loading coprocessor class " + className + " with path " + 
+        path + " and priority " + priority);
 
     // Have we already loaded the class, perhaps from an earlier region open
     // for the same table?
@@ -168,12 +169,15 @@ public abstract class CoprocessorHost<E extends CoprocessorEnvironment> {
       implClass = getClass().getClassLoader().loadClass(className);
     } catch (ClassNotFoundException e) {
       LOG.info("Class " + className + " needs to be loaded from a file - " +
-          path.toString() + ".");
+          path + ".");
       // go ahead to load from file system.
     }
 
     // If not, load
     if (implClass == null) {
+      if (path == null) {
+        throw new IOException("No jar path specified for " + className);
+      }
       // copy the jar to the local filesystem
       if (!path.toString().endsWith(".jar")) {
         throw new IOException(path.toString() + ": not a jar file?");
@@ -193,7 +197,6 @@ public abstract class CoprocessorHost<E extends CoprocessorEnvironment> {
          aborts runaway user code */
 
       // load the jar and get the implementation main class
-      String cp = System.getProperty("java.class.path");
       // NOTE: Path.toURL is deprecated (toURI instead) but the URLClassLoader
       // unsurprisingly wants URLs, not URIs; so we will use the deprecated
       // method which returns URLs for as long as it is available
@@ -215,11 +218,7 @@ public abstract class CoprocessorHost<E extends CoprocessorEnvironment> {
       }
       jarFile.close();
 
-      StringTokenizer st = new StringTokenizer(cp, File.pathSeparator);
+      ClassLoader cl = new CoprocessorClassLoader(paths,
-      while (st.hasMoreTokens()) {
-        paths.add((new File(st.nextToken())).getCanonicalFile().toURL());
-      }
-      ClassLoader cl = new URLClassLoader(paths.toArray(new URL[]{}),
         this.getClass().getClassLoader());
       Thread.currentThread().setContextClassLoader(cl);
       try {

hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java

@@ -61,11 +61,12 @@ public class TestClassLoading {
   static final String cpName3 = "TestCP3";
   static final String cpName4 = "TestCP4";
   static final String cpName5 = "TestCP5";
+  static final String cpName6 = "TestCP6";
 
-  private static Class regionCoprocessor1 = ColumnAggregationEndpoint.class;
+  private static Class<?> regionCoprocessor1 = ColumnAggregationEndpoint.class;
-  private static Class regionCoprocessor2 = GenericEndpoint.class;
+  private static Class<?> regionCoprocessor2 = GenericEndpoint.class;
-  private static Class regionServerCoprocessor = SampleRegionWALObserver.class;
+  private static Class<?> regionServerCoprocessor = SampleRegionWALObserver.class;
-  private static Class masterCoprocessor = BaseMasterObserver.class;
+  private static Class<?> masterCoprocessor = BaseMasterObserver.class;
 
   private static final String[] regionServerSystemCoprocessors =
       new String[]{
@@ -296,6 +297,37 @@ public class TestClassLoading {
     assertTrue("Class " + cpName3 + " was missing on a region", found);
   }
 
+  @Test
+  // HBASE-6308: Test CP classloader is the CoprocessorClassLoader
+  public void testPrivateClassLoader() throws Exception {
+    File jarFile = buildCoprocessorJar(cpName4);
+
+    // create a table that references the jar
+    HTableDescriptor htd = new HTableDescriptor(cpName4);
+    htd.addFamily(new HColumnDescriptor("test"));
+    htd.setValue("COPROCESSOR$1", jarFile.toString() + "|" + cpName4 + "|" +
+      Coprocessor.PRIORITY_USER);
+    HBaseAdmin admin = TEST_UTIL.getHBaseAdmin();
+    admin.createTable(htd);
+    TEST_UTIL.waitTableAvailable(htd.getName(), 5000);
+
+    // verify that the coprocessor was loaded correctly
+    boolean found = false;
+    MiniHBaseCluster hbase = TEST_UTIL.getHBaseCluster();
+    for (HRegion region:
+        hbase.getRegionServer(0).getOnlineRegionsLocalContext()) {
+      if (region.getRegionNameAsString().startsWith(cpName4)) {
+        Coprocessor cp = region.getCoprocessorHost().findCoprocessor(cpName4);
+        if (cp != null) {
+          found = true;
+          assertEquals("Class " + cpName4 + " was not loaded by CoprocessorClassLoader",
+            cp.getClass().getClassLoader().getClass(), CoprocessorClassLoader.class);
+        }
+      }
+    }
+    assertTrue("Class " + cpName4 + " was missing on a region", found);
+  }
+
   @Test
   // HBase-3810: Registering a Coprocessor at HTableDescriptor should be
   // less strict
@@ -304,8 +336,8 @@ public class TestClassLoading {
 
     File jarFile1 = buildCoprocessorJar(cpName1);
     File jarFile2 = buildCoprocessorJar(cpName2);
-    File jarFile4 = buildCoprocessorJar(cpName4);
     File jarFile5 = buildCoprocessorJar(cpName5);
+    File jarFile6 = buildCoprocessorJar(cpName6);
 
     String cpKey1 = "COPROCESSOR$1";
     String cpKey2 = " Coprocessor$2 ";
@@ -328,13 +360,13 @@ public class TestClassLoading {
     htd.setValue(cpKey3, cpValue3);
 
     // add 2 coprocessor by using new htd.addCoprocessor() api
-    htd.addCoprocessor(cpName4, new Path(jarFile4.getPath()),
+    htd.addCoprocessor(cpName5, new Path(jarFile5.getPath()),
         Coprocessor.PRIORITY_USER, null);
     Map<String, String> kvs = new HashMap<String, String>();
     kvs.put("k1", "v1");
     kvs.put("k2", "v2");
     kvs.put("k3", "v3");
-    htd.addCoprocessor(cpName5, new Path(jarFile5.getPath()),
+    htd.addCoprocessor(cpName6, new Path(jarFile6.getPath()),
         Coprocessor.PRIORITY_USER, kvs);
 
     HBaseAdmin admin = TEST_UTIL.getHBaseAdmin();
@@ -349,9 +381,9 @@ public class TestClassLoading {
 
     // verify that the coprocessor was loaded
     boolean found_2 = false, found_1 = false, found_3 = false,
-        found_4 = false, found_5 = false;
+        found_5 = false, found_6 = false;
-    boolean found5_k1 = false, found5_k2 = false, found5_k3 = false,
+    boolean found6_k1 = false, found6_k2 = false, found6_k3 = false,
-        found5_k4 = false;
+        found6_k4 = false;
 
     MiniHBaseCluster hbase = TEST_UTIL.getHBaseCluster();
     for (HRegion region:
@@ -364,17 +396,17 @@ public class TestClassLoading {
         found_3 = found_3 ||
             (region.getCoprocessorHost().findCoprocessor("SimpleRegionObserver")
                 != null);
-        found_4 = found_4 ||
+        found_5 = found_5 ||
-            (region.getCoprocessorHost().findCoprocessor(cpName4) != null);
+            (region.getCoprocessorHost().findCoprocessor(cpName5) != null);
 
         CoprocessorEnvironment env =
-            region.getCoprocessorHost().findCoprocessorEnvironment(cpName5);
+            region.getCoprocessorHost().findCoprocessorEnvironment(cpName6);
         if (env != null) {
-          found_5 = true;
+          found_6 = true;
           Configuration conf = env.getConfiguration();
-          found5_k1 = conf.get("k1") != null;
+          found6_k1 = conf.get("k1") != null;
-          found5_k2 = conf.get("k2") != null;
+          found6_k2 = conf.get("k2") != null;
-          found5_k3 = conf.get("k3") != null;
+          found6_k3 = conf.get("k3") != null;
         }
       }
     }
@@ -382,13 +414,13 @@ public class TestClassLoading {
     assertTrue("Class " + cpName1 + " was missing on a region", found_1);
     assertTrue("Class " + cpName2 + " was missing on a region", found_2);
     assertTrue("Class SimpleRegionObserver was missing on a region", found_3);
-    assertTrue("Class " + cpName4 + " was missing on a region", found_4);
     assertTrue("Class " + cpName5 + " was missing on a region", found_5);
+    assertTrue("Class " + cpName6 + " was missing on a region", found_6);
 
-    assertTrue("Configuration key 'k1' was missing on a region", found5_k1);
+    assertTrue("Configuration key 'k1' was missing on a region", found6_k1);
-    assertTrue("Configuration key 'k2' was missing on a region", found5_k2);
+    assertTrue("Configuration key 'k2' was missing on a region", found6_k2);
-    assertTrue("Configuration key 'k3' was missing on a region", found5_k3);
+    assertTrue("Configuration key 'k3' was missing on a region", found6_k3);
-    assertFalse("Configuration key 'k4' wasn't configured", found5_k4);
+    assertFalse("Configuration key 'k4' wasn't configured", found6_k4);
   }
 
   @Test