kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ewe...@apache.org
Subject [kafka] branch 1.0 updated: KAFKA-6277: Ensure loadClass for plugin class loaders is thread-safe.
Date Mon, 22 Jan 2018 03:41:48 GMT
This is an automated email from the ASF dual-hosted git repository.

ewencp pushed a commit to branch 1.0
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/1.0 by this push:
     new a8e3a9f  KAFKA-6277: Ensure loadClass for plugin class loaders is thread-safe.
a8e3a9f is described below

commit a8e3a9f81eb5fbc10f023d3876b9c7520755502e
Author: Konstantine Karantasis <konstantine@confluent.io>
AuthorDate: Sun Jan 21 19:41:20 2018 -0800

    KAFKA-6277: Ensure loadClass for plugin class loaders is thread-safe.
    
    `loadClass` needs to be synchronized to protect subsequent calls to `defineClass`.
    
    Details in the javadoc of this PR as well as here too: https://docs.oracle.com/javase/7/docs/technotes/guides/lang/cl-mt.html
    
    /cc ewencp rhauch
    
    Author: Konstantine Karantasis <konstantine@confluent.io>
    
    Reviewers: Ewen Cheslack-Postava <ewen@confluent.io>
    
    Closes #4428 from kkonstantine/KAFKA-6277-Make-loadClass-thread-safe-for-class-loaders-of-Connect-plugins
    
    (cherry picked from commit 0fa52644debfbc20cda0a93678140537fa2cb24c)
    Signed-off-by: Ewen Cheslack-Postava <me@ewencp.org>
---
 .../runtime/isolation/PluginClassLoader.java       | 75 +++++++++++++++++-----
 1 file changed, 59 insertions(+), 16 deletions(-)

diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginClassLoader.java
b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginClassLoader.java
index 780ebd0..9a4fed2 100644
--- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginClassLoader.java
+++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginClassLoader.java
@@ -22,20 +22,56 @@ import org.slf4j.LoggerFactory;
 import java.net.URL;
 import java.net.URLClassLoader;
 
+/**
+ * A custom classloader dedicated to loading Connect plugin classes in classloading isolation.
+ * <p>
+ * Under the current scheme for classloading isolation in Connect, a plugin classloader loads
the
+ * classes that it finds in its urls. For classes that are either not found or are not supposed
to
+ * be loaded in isolation, this plugin classloader delegates their loading to its parent.
This makes
+ * this classloader a child-first classloader.
+ * <p>
+ * This class is thread-safe.
+ */
 public class PluginClassLoader extends URLClassLoader {
     private static final Logger log = LoggerFactory.getLogger(PluginClassLoader.class);
     private final URL pluginLocation;
 
+    static {
+        ClassLoader.registerAsParallelCapable();
+    }
+
+    /**
+     * Constructor that accepts a specific classloader as parent.
+     *
+     * @param pluginLocation the top-level location of the plugin to be loaded in isolation
by this
+     * classloader.
+     * @param urls the list of urls from which to load classes and resources for this plugin.
+     * @param parent the parent classloader to be used for delegation for classes that were
+     * not found or should not be loaded in isolation by this classloader.
+     */
     public PluginClassLoader(URL pluginLocation, URL[] urls, ClassLoader parent) {
         super(urls, parent);
         this.pluginLocation = pluginLocation;
     }
 
+    /**
+     * Constructor that defines the system classloader as parent of this plugin classloader.
+     *
+     * @param pluginLocation the top-level location of the plugin to be loaded in isolation
by this
+     * classloader.
+     * @param urls the list of urls from which to load classes and resources for this plugin.
+     */
     public PluginClassLoader(URL pluginLocation, URL[] urls) {
         super(urls);
         this.pluginLocation = pluginLocation;
     }
 
+    /**
+     * Returns the top-level location of the classes and dependencies required by the plugin
that
+     * is loaded by this classloader.
+     *
+     * @return the plugin location.
+     */
     public String location() {
         return pluginLocation.toString();
     }
@@ -45,25 +81,32 @@ public class PluginClassLoader extends URLClassLoader {
         return "PluginClassLoader{pluginLocation=" + pluginLocation + "}";
     }
 
+    // This method needs to be thread-safe because it is supposed to be called by multiple
+    // Connect tasks. While findClass is thread-safe, defineClass called within loadClass
of the
+    // base method is not. More on multithreaded classloaders in:
+    // https://docs.oracle.com/javase/7/docs/technotes/guides/lang/cl-mt.html
     @Override
-    protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException
{
-        Class<?> klass = findLoadedClass(name);
-        if (klass == null) {
-            try {
-                if (PluginUtils.shouldLoadInIsolation(name)) {
-                    klass = findClass(name);
+    protected synchronized Class<?> loadClass(String name, boolean resolve)
+            throws ClassNotFoundException {
+        synchronized (getClassLoadingLock(name)) {
+            Class<?> klass = findLoadedClass(name);
+            if (klass == null) {
+                try {
+                    if (PluginUtils.shouldLoadInIsolation(name)) {
+                        klass = findClass(name);
+                    }
+                } catch (ClassNotFoundException e) {
+                    // Not found in loader's path. Search in parents.
+                    log.trace("Class '{}' not found. Delegating to parent", name);
                 }
-            } catch (ClassNotFoundException e) {
-                // Not found in loader's path. Search in parents.
-                log.trace("Class '{}' not found. Delegating to parent", name);
             }
+            if (klass == null) {
+                klass = super.loadClass(name, false);
+            }
+            if (resolve) {
+                resolveClass(klass);
+            }
+            return klass;
         }
-        if (klass == null) {
-            klass = super.loadClass(name, false);
-        }
-        if (resolve) {
-            resolveClass(klass);
-        }
-        return klass;
     }
 }

-- 
To stop receiving notification emails like this one, please contact
ewencp@apache.org.

Mime
View raw message