kafka-jira mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (KAFKA-6277) Make loadClass thread-safe for class loaders of Connect plugins
Date Wed, 17 Jan 2018 04:07:03 GMT

    [ https://issues.apache.org/jira/browse/KAFKA-6277?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16328247#comment-16328247
] 

ASF GitHub Bot commented on KAFKA-6277:
---------------------------------------

ewencp closed pull request #4428: KAFKA-6277: Ensure loadClass for plugin class loaders is
thread-safe.
URL: https://github.com/apache/kafka/pull/4428
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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 780ebd09bfc..7ac3fe3e886 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,50 @@
 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 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.
+ */
 public class PluginClassLoader extends URLClassLoader {
     private static final Logger log = LoggerFactory.getLogger(PluginClassLoader.class);
     private final URL pluginLocation;
 
+    /**
+     * 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,8 +75,13 @@ public String toString() {
         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
{
+    protected synchronized Class<?> loadClass(String name, boolean resolve)
+            throws ClassNotFoundException {
         Class<?> klass = findLoadedClass(name);
         if (klass == null) {
             try {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> Make loadClass thread-safe for class loaders of Connect plugins
> ---------------------------------------------------------------
>
>                 Key: KAFKA-6277
>                 URL: https://issues.apache.org/jira/browse/KAFKA-6277
>             Project: Kafka
>          Issue Type: Bug
>          Components: KafkaConnect
>    Affects Versions: 1.0.0, 0.11.0.2
>            Reporter: Konstantine Karantasis
>            Assignee: Konstantine Karantasis
>            Priority: Major
>             Fix For: 1.0.1, 0.11.0.3
>
>
> In Connect's classloading isolation framework, {{PluginClassLoader}} class encounters
a race condition when several threads corresponding to tasks using a specific plugin (e.g.
a Connector) try to load the same class at the same time on a single JVM. 
> The race condition is related to calls to method {{defineClass}} which, contract to {{findClass}},
is not thread safe for classloaders that override {{loadClass}}. More details here: 
> https://docs.oracle.com/javase/7/docs/technotes/guides/lang/cl-mt.html



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message