You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2011/09/16 01:11:48 UTC

svn commit: r1171325 - in /tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic: PlasticClassLoader.java PlasticClassPool.java

Author: hlship
Date: Thu Sep 15 23:11:48 2011
New Revision: 1171325

URL: http://svn.apache.org/viewvc?rev=1171325&view=rev
Log:
Revert "TAP5-1650: On a cold start with a large number of incoming requests, Tapestry can deadlock inside PlasticClassLoader/PlasticClassPool"

This reverts commit 3bc03edef7c8c4839ef833b45de24251b3ad52ac.

Modified:
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassLoader.java
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java

Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassLoader.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassLoader.java?rev=1171325&r1=1171324&r2=1171325&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassLoader.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassLoader.java Thu Sep 15 23:11:48 2011
@@ -14,13 +14,8 @@
 
 package org.apache.tapestry5.internal.plastic;
 
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
-
 public class PlasticClassLoader extends ClassLoader
 {
-    final Lock classloaderLock = new ReentrantLock();
-
     private final ClassLoaderDelegate delegate;
 
     public PlasticClassLoader(ClassLoader parent, ClassLoaderDelegate delegate)
@@ -31,32 +26,25 @@ public class PlasticClassLoader extends 
     }
 
     @Override
-    protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException
+    protected synchronized Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException
     {
-        classloaderLock.lock();
+        Class<?> loadedClass = findLoadedClass(name);
 
-        try
+        if (loadedClass != null)
+            return loadedClass;
+
+        if (delegate.shouldInterceptClassLoading(name))
         {
-            Class<?> loadedClass = findLoadedClass(name);
+            Class<?> c = delegate.loadAndTransformClass(name);
 
-            if (loadedClass != null)
-                return loadedClass;
+            if (resolve)
+                resolveClass(c);
 
-            if (delegate.shouldInterceptClassLoading(name))
-            {
-                Class<?> c = delegate.loadAndTransformClass(name);
-
-                if (resolve)
-                    resolveClass(c);
-
-                return c;
-            } else
-            {
-                return super.loadClass(name, resolve);
-            }
-        } finally
+            return c;
+        }
+        else
         {
-            classloaderLock.unlock();
+            return super.loadClass(name, resolve);
         }
     }
 

Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java?rev=1171325&r1=1171324&r2=1171325&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java Thu Sep 15 23:11:48 2011
@@ -14,17 +14,30 @@
 
 package org.apache.tapestry5.internal.plastic;
 
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
+
 import org.apache.tapestry5.internal.plastic.asm.ClassReader;
 import org.apache.tapestry5.internal.plastic.asm.ClassWriter;
 import org.apache.tapestry5.internal.plastic.asm.Opcodes;
 import org.apache.tapestry5.internal.plastic.asm.tree.AnnotationNode;
 import org.apache.tapestry5.internal.plastic.asm.tree.ClassNode;
-import org.apache.tapestry5.plastic.*;
-
-import java.lang.annotation.Annotation;
-import java.lang.reflect.Modifier;
-import java.util.*;
-import java.util.concurrent.CopyOnWriteArrayList;
+import org.apache.tapestry5.plastic.AnnotationAccess;
+import org.apache.tapestry5.plastic.ClassInstantiator;
+import org.apache.tapestry5.plastic.ClassType;
+import org.apache.tapestry5.plastic.PlasticClassEvent;
+import org.apache.tapestry5.plastic.PlasticClassListener;
+import org.apache.tapestry5.plastic.PlasticClassListenerHub;
+import org.apache.tapestry5.plastic.PlasticClassTransformation;
+import org.apache.tapestry5.plastic.PlasticManagerDelegate;
+import org.apache.tapestry5.plastic.TransformationOption;
 
 /**
  * Responsible for managing a class loader that allows ASM {@link ClassNode}s
@@ -71,9 +84,7 @@ public class PlasticClassPool implements
         }
     }
 
-    /**
-     * Map from FQCN to BaseClassDef.
-     */
+    /** Map from FQCN to BaseClassDef. */
     private final Map<String, BaseClassDef> baseClassDefs = new HashMap<String, PlasticClassPool.BaseClassDef>();
 
     private final Set<TransformationOption> options;
@@ -81,14 +92,18 @@ public class PlasticClassPool implements
     /**
      * Creates the pool with a set of controlled packages; all classes in the controlled packages are loaded by the
      * pool's class loader, and all top-level classes in the controlled packages are transformed via the delegate.
-     *
-     * @param parentLoader       typically, the Thread's context class loader
-     * @param delegate           responsible for end stages of transforming top-level classes
-     * @param controlledPackages set of package names (note: retained, not copied)
-     * @param options            used when transforming classes
+     * 
+     * @param parentLoader
+     *            typically, the Thread's context class loader
+     * @param delegate
+     *            responsible for end stages of transforming top-level classes
+     * @param controlledPackages
+     *            set of package names (note: retained, not copied)
+     * @param options
+     *            used when transforming classes
      */
     public PlasticClassPool(ClassLoader parentLoader, PlasticManagerDelegate delegate, Set<String> controlledPackages,
-                            Set<TransformationOption> options)
+            Set<TransformationOption> options)
     {
         loader = new PlasticClassLoader(parentLoader, this);
         this.delegate = delegate;
@@ -102,7 +117,7 @@ public class PlasticClassPool implements
     }
 
     public synchronized Class realizeTransformedClass(ClassNode classNode, InheritanceData inheritanceData,
-                                                      StaticContext staticContext)
+            StaticContext staticContext)
     {
         Class result = realize(PlasticInternalUtils.toClassName(classNode.name), ClassType.PRIMARY, classNode);
 
@@ -126,7 +141,7 @@ public class PlasticClassPool implements
     }
 
     private PlasticClassEvent toEvent(final String primaryClassName, final ClassType classType,
-                                      final ClassNode classNode)
+            final ClassNode classNode)
     {
         return new PlasticClassEvent()
         {
@@ -189,7 +204,8 @@ public class PlasticClassPool implements
                     return annotationType.cast(searchClass.getAnnotation(annotationType));
                 }
             };
-        } catch (Exception ex)
+        }
+        catch (Exception ex)
         {
             throw new RuntimeException(ex);
         }
@@ -250,7 +266,8 @@ public class PlasticClassPool implements
         try
         {
             return loader.loadClass(className);
-        } catch (Exception ex)
+        }
+        catch (Exception ex)
         {
             throw new RuntimeException(String.format("Unable to load class %s: %s", className,
                     PlasticInternalUtils.toMessage(ex)), ex);
@@ -320,7 +337,7 @@ public class PlasticClassPool implements
     /**
      * For a fully-qualified class name of an <em>existing</em> class, loads the bytes for the class
      * and returns a PlasticClass instance.
-     *
+     * 
      * @throws ClassNotFoundException
      */
     public InternalPlasticClassTransformation getPlasticClassTransformation(String className)
@@ -357,8 +374,9 @@ public class PlasticClassPool implements
     /**
      * Constructs a class node by reading the raw bytecode for a class and instantiating a ClassNode
      * (via {@link ClassReader#accept(org.apache.tapestry5.internal.plastic.asm.ClassVisitor, int)}).
-     *
-     * @param className fully qualified class name
+     * 
+     * @param className
+     *            fully qualified class name
      * @return corresponding ClassNode
      */
     public ClassNode constructClassNode(String className)
@@ -388,7 +406,8 @@ public class PlasticClassPool implements
                     PlasticInternalUtils.toInternalName(baseClassName), null);
 
             return createTransformation(baseClassName, newClassNode);
-        } catch (ClassNotFoundException ex)
+        }
+        catch (ClassNotFoundException ex)
         {
             throw new RuntimeException(String.format("Unable to create class %s as sub-class of %s: %s", newClassName,
                     baseClassName, PlasticInternalUtils.toMessage(ex)), ex);
@@ -397,57 +416,48 @@ public class PlasticClassPool implements
 
     public synchronized ClassInstantiator getClassInstantiator(String className)
     {
-        ClassInstantiator result;
-
-        loader.classloaderLock.lock();
-
-        try
+        if (!instantiators.containsKey(className))
         {
-            if (!instantiators.containsKey(className))
+            try
             {
-                try
-                {
-                    loader.loadClass(className);
-                } catch (ClassNotFoundException ex)
-                {
-                    throw new RuntimeException(ex);
-                }
+                loader.loadClass(className);
+            }
+            catch (ClassNotFoundException ex)
+            {
+                throw new RuntimeException(ex);
             }
-
-            result = instantiators.get(className);
-        } finally
-        {
-            loader.classloaderLock.unlock();
         }
 
-        if (result != null)
+        ClassInstantiator result = instantiators.get(className);
+
+        if (result == null)
         {
-            return result;
-        }
+            // TODO: Verify that the problem is incorrect package, and not any other failure.
 
-        // TODO: Verify that the problem is incorrect package, and not any other failure.
+            StringBuilder b = new StringBuilder();
+            b.append("Class '")
+                    .append(className)
+                    .append("' is not a transformed class. Transformed classes should be in one of the following packages: ");
 
-        StringBuilder b = new StringBuilder();
-        b.append("Class '")
-                .append(className)
-                .append("' is not a transformed class. Transformed classes should be in one of the following packages: ");
+            String sep = "";
 
-        String sep = "";
+            List<String> names = new ArrayList<String>(controlledPackages);
+            Collections.sort(names);
 
-        List<String> names = new ArrayList<String>(controlledPackages);
-        Collections.sort(names);
+            for (String name : names)
+            {
+                b.append(sep);
+                b.append(name);
 
-        for (String name : names)
-        {
-            b.append(sep);
-            b.append(name);
+                sep = ", ";
+            }
 
-            sep = ", ";
-        }
+            String message = b.append(".").toString();
 
-        String message = b.append(".").toString();
+            throw new IllegalArgumentException(message);
+        }
 
-        throw new IllegalArgumentException(message);
+        return result;
     }
 
     TypeCategory getTypeCategory(String typeName)