Repository: pinpoint-apm/pinpoint
Issue: Potential deadlock with PluginClassLoader
Labels: bug

When asynchronously loading classes and applying interceptors, deadlock may happen when the interceptor itself references a class loaded with PluginClassLoader.

One thread locks on the system class loader and waits to lock the plugin class loader (when applying an interceptor), and another thread locks on the plugin class loader and waits to lock the system class loader.


Issue comments:
resolved in #404 


Pull request review comments:

Patch that resolved the issue:
diff --git a/commons/src/main/java/com/navercorp/pinpoint/common/plugin/PluginLoader.java b/commons/src/main/java/com/navercorp/pinpoint/common/plugin/PluginLoader.java
index c91a4c20797c..d5e4e572bf69 100644
--- a/commons/src/main/java/com/navercorp/pinpoint/common/plugin/PluginLoader.java
+++ b/commons/src/main/java/com/navercorp/pinpoint/common/plugin/PluginLoader.java
@@ -47,15 +47,15 @@ public static <T> List<T> load(Class<T> serviceType, URL[] urls) {
         return load(serviceType, classLoader);
     }
 
-    private static PluginClassLoader createPluginClassLoader(final URL[] urls, final ClassLoader parent) {
+    private static PluginLoaderClassLoader createPluginClassLoader(final URL[] urls, final ClassLoader parent) {
         if (SECURITY_MANAGER != null) {
-            return AccessController.doPrivileged(new PrivilegedAction<PluginClassLoader>() {
-                public PluginClassLoader run() {
-                    return new PluginClassLoader(urls, parent);
+            return AccessController.doPrivileged(new PrivilegedAction<PluginLoaderClassLoader>() {
+                public PluginLoaderClassLoader run() {
+                    return new PluginLoaderClassLoader(urls, parent);
                 }
             });
         } else {
-            return new PluginClassLoader(urls, parent);
+            return new PluginLoaderClassLoader(urls, parent);
         }
     }
     
diff --git a/commons/src/main/java/com/navercorp/pinpoint/common/plugin/PluginClassLoader.java b/commons/src/main/java/com/navercorp/pinpoint/common/plugin/PluginLoaderClassLoader.java
similarity index 80%
rename from commons/src/main/java/com/navercorp/pinpoint/common/plugin/PluginClassLoader.java
rename to commons/src/main/java/com/navercorp/pinpoint/common/plugin/PluginLoaderClassLoader.java
index 20d7d611d3b4..31c9cf00f2e6 100644
--- a/commons/src/main/java/com/navercorp/pinpoint/common/plugin/PluginClassLoader.java
+++ b/commons/src/main/java/com/navercorp/pinpoint/common/plugin/PluginLoaderClassLoader.java
@@ -23,19 +23,19 @@
 /**
  * @author emeroad
  */
-public class PluginClassLoader extends URLClassLoader {
+public class PluginLoaderClassLoader extends URLClassLoader {
 //      for debug
 //    private final String loaderName
 
-    public PluginClassLoader(URL[] urls, ClassLoader parent) {
+    public PluginLoaderClassLoader(URL[] urls, ClassLoader parent) {
         super(urls, parent);
     }
 
-    public PluginClassLoader(URL[] urls) {
+    public PluginLoaderClassLoader(URL[] urls) {
         super(urls);
     }
 
-    public PluginClassLoader(URL[] urls, ClassLoader parent, URLStreamHandlerFactory factory) {
+    public PluginLoaderClassLoader(URL[] urls, ClassLoader parent, URLStreamHandlerFactory factory) {
         super(urls, parent, factory);
     }
 
diff --git a/profiler-optional/src/main/java/com/navercorp/pinpoint/profiler/plugin/Java7PluginClassLoader.java b/profiler-optional/src/main/java/com/navercorp/pinpoint/profiler/plugin/Java7PluginClassLoader.java
new file mode 100644
index 000000000000..2e4fea3a25a4
--- /dev/null
+++ b/profiler-optional/src/main/java/com/navercorp/pinpoint/profiler/plugin/Java7PluginClassLoader.java
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2014 NAVER Corp.
+ *
+ * Licensed 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 com.navercorp.pinpoint.profiler.plugin;
+
+import java.net.URL;
+import java.net.URLClassLoader;
+
+/**
+ * @author Jongho Moon
+ * @author emeroad
+ */
+public class Java7PluginClassLoader extends URLClassLoader {
+    static {
+        ClassLoader.registerAsParallelCapable();
+    }
+    
+    public Java7PluginClassLoader(URL[] urls, ClassLoader parent) {
+        super(urls, parent);
+    }
+
+    @Override
+    protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+        // for debugging
+        return super.loadClass(name, resolve);
+    }
+}
diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/DefaultPluginClassLoaderFactory.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/DefaultPluginClassLoaderFactory.java
index 51627eea42c0..d847d810e9b3 100644
--- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/DefaultPluginClassLoaderFactory.java
+++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/DefaultPluginClassLoaderFactory.java
@@ -18,6 +18,7 @@
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.lang.reflect.Constructor;
 import java.net.URL;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
@@ -27,18 +28,40 @@
 import java.util.logging.Logger;
 
 import com.navercorp.pinpoint.bootstrap.plugin.PluginClassLoaderFactory;
-import com.navercorp.pinpoint.common.plugin.PluginClassLoader;
+import com.navercorp.pinpoint.exception.PinpointException;
 
 public class DefaultPluginClassLoaderFactory implements PluginClassLoaderFactory {
 
-    private final Logger logger = Logger.getLogger(DefaultPluginClassLoaderFactory.class.getName());
-
     private static final SecurityManager SECURITY_MANAGER = System.getSecurityManager();
+    private static final Constructor<?> CLASS_LOADER_CONSTRUCTOR;
+    
+    static {
+        boolean java6 = false;
+        
+        try {
+            ClassLoader.class.getDeclaredMethod("registerAsParallelCapable");
+        } catch (Exception e) {
+            java6 = true;
+        }
+
+        String pluginClassLoaderName = java6 ? "com.navercorp.pinpoint.profiler.plugin.Java6PluginClassLoader" : "com.navercorp.pinpoint.profiler.plugin.Java7PluginClassLoader";
+        
+        try {
+            Class<?> pluginClassLoaderType = Class.forName(pluginClassLoaderName);
+            CLASS_LOADER_CONSTRUCTOR = pluginClassLoaderType.getDeclaredConstructor(URL[].class, ClassLoader.class);
+        } catch (ClassNotFoundException e) {
+            throw new NoClassDefFoundError("Cannot find plugin class loader: " + pluginClassLoaderName);
+        } catch (Exception e) {
+            throw new PinpointException("Failed to prepare " + pluginClassLoaderName, e);
+        }
+    }
 
+    private final Logger logger = Logger.getLogger(DefaultPluginClassLoaderFactory.class.getName());
     private final URL[] pluginJars;
     private final ConcurrentHashMap<ClassLoader, ClassLoader> cache = new ConcurrentHashMap<ClassLoader, ClassLoader>();
     private final AtomicReference<ClassLoader> forBootstrapClassLoader = new AtomicReference<ClassLoader>();
     
+    
     public DefaultPluginClassLoaderFactory(URL[] pluginJars) {
         this.pluginJars = pluginJars;
     }
@@ -90,15 +113,23 @@ private ClassLoader getForBootstrap() {
         }
     }
 
-    private PluginClassLoader createPluginClassLoader(final URL[] urls, final ClassLoader parent) {
+    private ClassLoader createPluginClassLoader(final URL[] urls, final ClassLoader parent) {
         if (SECURITY_MANAGER != null) {
-            return AccessController.doPrivileged(new PrivilegedAction<PluginClassLoader>() {
-                public PluginClassLoader run() {
-                    return new PluginClassLoader(urls, parent);
+            return AccessController.doPrivileged(new PrivilegedAction<ClassLoader>() {
+                public ClassLoader run() {
+                    return createPluginClassLoader0(urls, parent);
                 }
             });
         } else {
-            return new PluginClassLoader(urls, parent);
+            return createPluginClassLoader0(urls, parent);
+        }
+    }
+    
+    private ClassLoader createPluginClassLoader0(URL[] urls, ClassLoader parent) {
+        try {
+            return (ClassLoader)CLASS_LOADER_CONSTRUCTOR.newInstance(urls, parent);
+        } catch (Exception e) {
+            throw new PinpointException("Failed to create plugin classloader instance", e);
         }
     }
 
@@ -111,7 +142,7 @@ private void close(ClassLoader classLoader) {
             try {
                 ((Closeable)classLoader).close();
             } catch (IOException e) {
-                logger.log(Level.WARNING, "PluginClassLoader.close() fail. Caused:" + e.getMessage(), e);
+                logger.log(Level.WARNING, "Java6PluginClassLoader.close() fail. Caused:" + e.getMessage(), e);
             }
         }
     }
diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/Java6PluginClassLoader.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/Java6PluginClassLoader.java
new file mode 100644
index 000000000000..799eb9c5ef1f
--- /dev/null
+++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/Java6PluginClassLoader.java
@@ -0,0 +1,93 @@
+/*
+ * Copyright 2014 NAVER Corp.
+ *
+ * Licensed 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 com.navercorp.pinpoint.profiler.plugin;
+
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.concurrent.locks.ReentrantLock;
+
+/**
+ * @author Jongho Moon
+ * @author emeroad
+ */
+public class Java6PluginClassLoader extends URLClassLoader {
+
+    private final ReentrantLock lock = new ReentrantLock();
+
+    public Java6PluginClassLoader(URL[] urls, ClassLoader parent) {
+        super(urls, parent);
+    }
+
+    @Override
+    protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+        ClassLoader parent = getParent();
+        
+        while (!lock.tryLock()) {
+            try {
+                try {
+                    this.wait();
+                } catch (IllegalMonitorStateException e) {
+                    try {
+                        parent.wait();
+                    } catch (IllegalMonitorStateException e2) {
+                        // should sleep?
+                    }
+                }
+            } catch (InterruptedException e) {
+                throw new RuntimeException("Interrupted", e);
+            }
+        }
+        
+        Class<?> c = null;
+
+        try {
+
+            synchronized (this) {
+                c = findLoadedClass(name);
+
+                if (c == null) {
+                    try {
+                        c = parent.loadClass(name);
+                    } catch (ClassNotFoundException e) {
+
+                    }
+
+                    if (c == null) {
+                        c = findClass(name);
+                    }
+                }
+
+                if (resolve) {
+                    resolveClass(c);
+                }
+            }
+            
+
+        } finally {
+            lock.unlock();
+        }
+
+        synchronized (this) {
+            this.notify();
+        }
+        
+        synchronized (parent) {
+            parent.notify();
+        }
+        return c;
+    }
+}


Test patch: