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/17 01:50:36 UTC

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

Author: hlship
Date: Fri Sep 16 23:50:35 2011
New Revision: 1171858

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

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=1171858&r1=1171857&r2=1171858&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 Fri Sep 16 23:50:35 2011
@@ -41,14 +41,13 @@ public class PlasticClassLoader extends 
                 resolveClass(c);
 
             return c;
-        }
-        else
+        } else
         {
             return super.loadClass(name, resolve);
         }
     }
 
-    Class<?> defineClassWithBytecode(String className, byte[] bytecode)
+    synchronized Class<?> defineClassWithBytecode(String className, byte[] bytecode)
     {
         return defineClass(className, bytecode, 0, bytecode.length);
     }

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=1171858&r1=1171857&r2=1171858&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 Fri Sep 16 23:50:35 2011
@@ -14,30 +14,17 @@
 
 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.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;
+import org.apache.tapestry5.plastic.*;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Modifier;
+import java.util.*;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * Responsible for managing a class loader that allows ASM {@link ClassNode}s
@@ -52,6 +39,10 @@ public class PlasticClassPool implements
 
     private final Set<String> controlledPackages;
 
+    /**
+     * Maps class names to instantiators for that class name.
+     * Synchronized on the loader.
+     */
     private final Map<String, ClassInstantiator> instantiators = PlasticInternalUtils.newMap();
 
     private final InheritanceData emptyInheritanceData = new InheritanceData();
@@ -84,7 +75,9 @@ public class PlasticClassPool implements
         }
     }
 
-    /** Map from FQCN to BaseClassDef. */
+    /**
+     * Map from FQCN to BaseClassDef. Synchronized on the loader.
+     */
     private final Map<String, BaseClassDef> baseClassDefs = new HashMap<String, PlasticClassPool.BaseClassDef>();
 
     private final Set<TransformationOption> options;
@@ -92,18 +85,14 @@ 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;
@@ -116,32 +105,39 @@ public class PlasticClassPool implements
         return loader;
     }
 
-    public synchronized Class realizeTransformedClass(ClassNode classNode, InheritanceData inheritanceData,
-            StaticContext staticContext)
+    public Class realizeTransformedClass(ClassNode classNode, InheritanceData inheritanceData,
+                                         StaticContext staticContext)
     {
-        Class result = realize(PlasticInternalUtils.toClassName(classNode.name), ClassType.PRIMARY, classNode);
+        synchronized (loader)
+        {
+            Class result = realize(PlasticInternalUtils.toClassName(classNode.name), ClassType.PRIMARY, classNode);
 
-        baseClassDefs.put(result.getName(), new BaseClassDef(inheritanceData, staticContext));
+            baseClassDefs.put(result.getName(), new BaseClassDef(inheritanceData, staticContext));
+
+            return result;
+        }
 
-        return result;
     }
 
-    public synchronized Class realize(String primaryClassName, ClassType classType, final ClassNode classNode)
+    public Class realize(String primaryClassName, ClassType classType, final ClassNode classNode)
     {
-        if (!listeners.isEmpty())
+        synchronized (loader)
         {
-            fire(toEvent(primaryClassName, classType, classNode));
-        }
+            if (!listeners.isEmpty())
+            {
+                fire(toEvent(primaryClassName, classType, classNode));
+            }
 
-        byte[] bytecode = toBytecode(classNode);
+            byte[] bytecode = toBytecode(classNode);
 
-        String className = PlasticInternalUtils.toClassName(classNode.name);
+            String className = PlasticInternalUtils.toClassName(classNode.name);
 
-        return loader.defineClassWithBytecode(className, bytecode);
+            return loader.defineClassWithBytecode(className, bytecode);
+        }
     }
 
     private PlasticClassEvent toEvent(final String primaryClassName, final ClassType classType,
-            final ClassNode classNode)
+                                      final ClassNode classNode)
     {
         return new PlasticClassEvent()
         {
@@ -188,7 +184,7 @@ public class PlasticClassPool implements
     {
         try
         {
-            final Class searchClass = loader.loadClass(className);
+            final Class<?> searchClass = loader.loadClass(className);
 
             return new AnnotationAccess()
             {
@@ -199,13 +195,10 @@ public class PlasticClassPool implements
 
                 public <T extends Annotation> T getAnnotation(Class<T> annotationType)
                 {
-                    // For the life of me, I don't understand why the cast is necessary.
-
-                    return annotationType.cast(searchClass.getAnnotation(annotationType));
+                    return searchClass.getAnnotation(annotationType);
                 }
             };
-        }
-        catch (Exception ex)
+        } catch (Exception ex)
         {
             throw new RuntimeException(ex);
         }
@@ -214,7 +207,9 @@ public class PlasticClassPool implements
     public AnnotationAccess createAnnotationAccess(List<AnnotationNode> annotationNodes)
     {
         if (annotationNodes == null)
+        {
             return EmptyAnnotationAccess.SINGLETON;
+        }
 
         final Map<String, Object> cache = PlasticInternalUtils.newMap();
         final Map<String, AnnotationNode> nameToNode = PlasticInternalUtils.newMap();
@@ -226,7 +221,6 @@ public class PlasticClassPool implements
 
         return new AnnotationAccess()
         {
-
             public <T extends Annotation> boolean hasAnnotation(Class<T> annotationType)
             {
                 return nameToNode.containsKey(annotationType.getName());
@@ -266,8 +260,7 @@ 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);
@@ -337,7 +330,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)
@@ -374,9 +367,8 @@ 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)
@@ -406,65 +398,69 @@ 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);
         }
     }
 
-    public synchronized ClassInstantiator getClassInstantiator(String className)
+    public ClassInstantiator getClassInstantiator(String className)
     {
-        if (!instantiators.containsKey(className))
+        synchronized (loader)
         {
-            try
+            if (!instantiators.containsKey(className))
             {
-                loader.loadClass(className);
+                try
+                {
+                    loader.loadClass(className);
+                } catch (ClassNotFoundException ex)
+                {
+                    throw new RuntimeException(ex);
+                }
             }
-            catch (ClassNotFoundException ex)
+
+            ClassInstantiator result = instantiators.get(className);
+
+            if (result == null)
             {
-                throw new RuntimeException(ex);
-            }
-        }
+                // TODO: Verify that the problem is incorrect package, and not any other failure.
 
-        ClassInstantiator result = instantiators.get(className);
+                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: ");
 
-        if (result == null)
-        {
-            // TODO: Verify that the problem is incorrect package, and not any other failure.
+                String sep = "";
 
-            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: ");
+                List<String> names = new ArrayList<String>(controlledPackages);
+                Collections.sort(names);
 
-            String sep = "";
+                for (String name : names)
+                {
+                    b.append(sep);
+                    b.append(name);
 
-            List<String> names = new ArrayList<String>(controlledPackages);
-            Collections.sort(names);
+                    sep = ", ";
+                }
 
-            for (String name : names)
-            {
-                b.append(sep);
-                b.append(name);
+                String message = b.append(".").toString();
 
-                sep = ", ";
+                throw new IllegalArgumentException(message);
             }
 
-            String message = b.append(".").toString();
-
-            throw new IllegalArgumentException(message);
+            return result;
         }
-
-        return result;
     }
 
     TypeCategory getTypeCategory(String typeName)
     {
-        // TODO: Is this the right place to cache this data?
+        synchronized (loader)
+        {
+            // TODO: Is this the right place to cache this data?
 
-        return typeName2Category.get(typeName);
+            return typeName2Category.get(typeName);
+        }
     }
 
     public void addPlasticClassListener(PlasticClassListener listener)