You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by ew...@apache.org on 2018/01/22 03:41:42 UTC
[kafka] branch trunk updated: KAFKA-6277: Ensure loadClass for
plugin class loaders is thread-safe.
This is an automated email from the ASF dual-hosted git repository.
ewencp pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/trunk by this push:
new 0fa5264 KAFKA-6277: Ensure loadClass for plugin class loaders is thread-safe.
0fa5264 is described below
commit 0fa52644debfbc20cda0a93678140537fa2cb24c
Author: Konstantine Karantasis <ko...@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 <ko...@confluent.io>
Reviewers: Ewen Cheslack-Postava <ew...@confluent.io>
Closes #4428 from kkonstantine/KAFKA-6277-Make-loadClass-thread-safe-for-class-loaders-of-Connect-plugins
---
.../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.