You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2011/09/16 01:11:36 UTC
svn commit: r1171322 - in /tapestry/tapestry5/trunk:
plastic/src/main/java/org/apache/tapestry5/internal/plastic/
plastic/src/main/java/org/apache/tapestry5/plastic/
tapestry-core/src/main/java/org/apache/tapestry5/internal/services/
tapestry-ioc/src/m...
Author: hlship
Date: Thu Sep 15 23:11:36 2011
New Revision: 1171322
URL: http://svn.apache.org/viewvc?rev=1171322&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 b33e13ed6ff474143676ccbbcdd6775019f6babb.
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
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/RegistryBuilder.java
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PlasticProxyFactoryImpl.java
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/services/PlasticProxyFactory.java
tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/DefaultModuleDefImplTest.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=1171322&r1=1171321&r2=1171322&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:36 2011
@@ -15,25 +15,25 @@
package org.apache.tapestry5.internal.plastic;
import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
public class PlasticClassLoader extends ClassLoader
{
- private final Lock classLoaderLock;
+ final Lock classloaderLock = new ReentrantLock();
private final ClassLoaderDelegate delegate;
- public PlasticClassLoader(ClassLoader parent, ClassLoaderDelegate delegate, Lock classLoaderLock)
+ public PlasticClassLoader(ClassLoader parent, ClassLoaderDelegate delegate)
{
super(parent);
this.delegate = delegate;
- this.classLoaderLock = classLoaderLock;
}
@Override
protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException
{
- classLoaderLock.lock();
+ classloaderLock.lock();
try
{
@@ -56,7 +56,7 @@ public class PlasticClassLoader extends
}
} finally
{
- classLoaderLock.unlock();
+ classloaderLock.unlock();
}
}
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=1171322&r1=1171321&r2=1171322&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:36 2011
@@ -61,7 +61,7 @@ public class PlasticClassPool implements
public Lock getClassLoaderLock()
{
- return classLoaderLock;
+ return loader.classloaderLock;
}
static class BaseClassDef
@@ -84,8 +84,6 @@ public class PlasticClassPool implements
private final Set<TransformationOption> options;
- private final Lock classLoaderLock;
-
/**
* 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.
@@ -94,17 +92,14 @@ public class PlasticClassPool implements
* @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 classLoaderLock
*/
public PlasticClassPool(ClassLoader parentLoader, PlasticManagerDelegate delegate, Set<String> controlledPackages,
- Set<TransformationOption> options, Lock classLoaderLock)
+ Set<TransformationOption> options)
{
+ loader = new PlasticClassLoader(parentLoader, this);
this.delegate = delegate;
this.controlledPackages = controlledPackages;
this.options = options;
- this.classLoaderLock = classLoaderLock;
-
- loader = new PlasticClassLoader(parentLoader, this, classLoaderLock);
}
public ClassLoader getClassLoader()
@@ -410,7 +405,7 @@ public class PlasticClassPool implements
{
ClassInstantiator result;
- classLoaderLock.lock();
+ loader.classloaderLock.lock();
try
{
@@ -428,7 +423,7 @@ public class PlasticClassPool implements
result = instantiators.get(className);
} finally
{
- classLoaderLock.unlock();
+ loader.classloaderLock.unlock();
}
if (result != null)
Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java?rev=1171322&r1=1171321&r2=1171322&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java Thu Sep 15 23:11:36 2011
@@ -23,7 +23,6 @@ import java.util.Collection;
import java.util.EnumSet;
import java.util.Set;
import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
/**
* Manages the internal class loaders and other logics necessary to load and transform existing classes,
@@ -36,8 +35,6 @@ public class PlasticManager implements P
{
private final PlasticClassPool pool;
- private final Lock classLoaderLock;
-
/**
* A builder object for configuring the PlasticManager before instantiating it. Assumes a no-op
* {@link PlasticManagerDelegate} and an empty set of controlled packages, which is appropriate
@@ -50,8 +47,6 @@ public class PlasticManager implements P
private PlasticManagerDelegate delegate = new NoopDelegate();
- private Lock classLoaderLock = new ReentrantLock();
-
private final Set<String> packages = PlasticInternalUtils.newSet();
private final Set<TransformationOption> options = EnumSet.noneOf(TransformationOption.class);
@@ -63,17 +58,6 @@ public class PlasticManager implements P
this.loader = loader;
}
- public PlasticManagerBuilder classLoaderLock(Lock lock)
- {
- assert lock != null;
-
- check();
-
- classLoaderLock = lock;
-
- return this;
- }
-
/**
* Sets the {@link PlasticManagerDelegate}, which is ultimately responsible for
* transforming classes loaded from controlled packages. The default delegate
@@ -120,7 +104,7 @@ public class PlasticManager implements P
{
lock();
- return new PlasticManager(loader, delegate, packages, options, classLoaderLock);
+ return new PlasticManager(loader, delegate, packages, options);
}
}
@@ -149,18 +133,15 @@ public class PlasticManager implements P
* @param controlledPackageNames defines the packages that are to be transformed; top-classes in these packages
* (or sub-packages) will be passed to the delegate for transformation
* @param options used when transforming classes
- * @param classLoaderLock
*/
private PlasticManager(ClassLoader parentClassLoader, PlasticManagerDelegate delegate,
- Set<String> controlledPackageNames, Set<TransformationOption> options, Lock classLoaderLock)
+ Set<String> controlledPackageNames, Set<TransformationOption> options)
{
assert parentClassLoader != null;
assert delegate != null;
assert controlledPackageNames != null;
- this.classLoaderLock = classLoaderLock;
-
- pool = new PlasticClassPool(parentClassLoader, delegate, controlledPackageNames, options, classLoaderLock);
+ pool = new PlasticClassPool(parentClassLoader, delegate, controlledPackageNames, options);
}
/**
@@ -268,12 +249,12 @@ public class PlasticManager implements P
private void unlock()
{
- classLoaderLock.unlock();
+ getClassloaderLock().unlock();
}
private void lock()
{
- classLoaderLock.lock();
+ getClassloaderLock().lock();
}
/**
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java?rev=1171322&r1=1171321&r2=1171322&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java Thu Sep 15 23:11:36 2011
@@ -67,8 +67,6 @@ public final class ComponentInstantiator
private final ClassLoader parent;
- private final Lock classLoaderLock;
-
private final ComponentClassTransformWorker2 transformerChain;
private final LoggerSource loggerSource;
@@ -111,7 +109,6 @@ public final class ComponentInstantiator
}
};
-
public ComponentInstantiatorSourceImpl(Logger logger,
LoggerSource loggerSource,
@@ -136,7 +133,6 @@ public final class ComponentInstantiator
InternalComponentInvalidationEventHub invalidationHub)
{
this.parent = proxyFactory.getClassLoader();
- classLoaderLock = proxyFactory.getClassLoaderLock();
this.transformerChain = transformerChain;
this.logger = logger;
this.loggerSource = loggerSource;
@@ -187,7 +183,7 @@ public final class ComponentInstantiator
private void initializeService()
{
PlasticManagerBuilder builder = PlasticManager.withClassLoader(parent).delegate(this)
- .packages(controlledPackageNames).classLoaderLock(classLoaderLock);
+ .packages(controlledPackageNames);
if (!productionMode)
{
@@ -200,7 +196,7 @@ public final class ComponentInstantiator
classFactory = new ClassFactoryImpl(manager.getClassLoader(), logger);
- proxyFactory = new PlasticProxyFactoryImpl(manager.getClassLoader(), logger, classLoaderLock);
+ proxyFactory = new PlasticProxyFactoryImpl(manager.getClassLoader(), logger);
classToInstantiator.clear();
classToModel.clear();
Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/RegistryBuilder.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/RegistryBuilder.java?rev=1171322&r1=1171321&r2=1171322&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/RegistryBuilder.java (original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/RegistryBuilder.java Thu Sep 15 23:11:36 2011
@@ -34,7 +34,6 @@ import java.lang.reflect.AnnotatedElemen
import java.util.Arrays;
import java.util.List;
import java.util.Set;
-import java.util.concurrent.locks.ReentrantLock;
/**
* Used to construct the IoC {@link org.apache.tapestry5.ioc.Registry}. This class is <em>not</em> thread-safe. The
@@ -84,7 +83,7 @@ public final class RegistryBuilder
Logger proxyFactoryLogger = loggerSource.getLogger(TapestryIOCModule.class.getName() + ".PlasticProxyFactory");
classFactory = new ClassFactoryImpl(this.classLoader, classFactoryLogger);
- proxyFactory = new PlasticProxyFactoryImpl(this.classLoader, proxyFactoryLogger, new ReentrantLock());
+ proxyFactory = new PlasticProxyFactoryImpl(this.classLoader, proxyFactoryLogger);
add(TapestryIOCModule.class);
}
Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PlasticProxyFactoryImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PlasticProxyFactoryImpl.java?rev=1171322&r1=1171321&r2=1171322&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PlasticProxyFactoryImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PlasticProxyFactoryImpl.java Thu Sep 15 23:11:36 2011
@@ -28,7 +28,6 @@ import java.lang.reflect.Constructor;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.util.List;
-import java.util.concurrent.locks.Lock;
public class PlasticProxyFactoryImpl implements PlasticProxyFactory
{
@@ -36,11 +35,11 @@ public class PlasticProxyFactoryImpl imp
private final ClassLoader loader;
- public PlasticProxyFactoryImpl(ClassLoader parentClassLoader, Logger logger, Lock classLoaderLock)
+ public PlasticProxyFactoryImpl(ClassLoader parentClassLoader, Logger logger)
{
this.loader = parentClassLoader;
- manager = PlasticManager.withClassLoader(parentClassLoader).classLoaderLock(classLoaderLock).create();
+ manager = PlasticManager.withClassLoader(parentClassLoader).create();
if (logger != null)
{
@@ -53,11 +52,6 @@ public class PlasticProxyFactoryImpl imp
return manager.getClassLoader();
}
- public Lock getClassLoaderLock()
- {
- return manager.getClassloaderLock();
- }
-
public <T> ClassInstantiator<T> createProxy(Class<T> interfaceType, PlasticClassTransformer callback)
{
return manager.createProxy(interfaceType, callback);
Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/services/PlasticProxyFactory.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/services/PlasticProxyFactory.java?rev=1171322&r1=1171321&r2=1171322&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/services/PlasticProxyFactory.java (original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/services/PlasticProxyFactory.java Thu Sep 15 23:11:36 2011
@@ -23,7 +23,6 @@ import org.apache.tapestry5.plastic.Plas
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
-import java.util.concurrent.locks.Lock;
/**
* A service used to create proxies of varying types. As a secondary concern, manages to identify the
@@ -40,11 +39,6 @@ public interface PlasticProxyFactory ext
ClassLoader getClassLoader();
/**
- * Returns the Lock used to serialize access to the class loader. When creating child class loaders, it may be necessary to use this lock to avoid deadlocks.
- */
- Lock getClassLoaderLock();
-
- /**
* Creates a proxy object that implements the indicated interface, then invokes the callback to further
* configure the proxy.
*
Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/DefaultModuleDefImplTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/DefaultModuleDefImplTest.java?rev=1171322&r1=1171321&r2=1171322&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/DefaultModuleDefImplTest.java (original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/DefaultModuleDefImplTest.java Thu Sep 15 23:11:36 2011
@@ -23,13 +23,13 @@ import org.apache.tapestry5.ioc.def.Serv
import org.apache.tapestry5.ioc.internal.services.ClassFactoryImpl;
import org.apache.tapestry5.ioc.internal.services.PlasticProxyFactoryImpl;
import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
-import org.apache.tapestry5.ioc.internal.util.DummyLock;
import org.apache.tapestry5.ioc.internal.util.InternalUtils;
import org.apache.tapestry5.ioc.services.ClassFab;
import org.apache.tapestry5.ioc.services.ClassFactory;
import org.apache.tapestry5.ioc.services.MethodSignature;
import org.apache.tapestry5.ioc.services.PlasticProxyFactory;
import org.apache.tapestry5.ioc.test.IOCTestCase;
+import static org.easymock.EasyMock.contains;
import org.slf4j.Logger;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
@@ -40,8 +40,6 @@ import java.lang.reflect.Modifier;
import java.util.List;
import java.util.Set;
-import static org.easymock.EasyMock.contains;
-
public class DefaultModuleDefImplTest extends IOCTestCase
{
private ClassFactory classFactory;
@@ -54,7 +52,7 @@ public class DefaultModuleDefImplTest ex
public void setup()
{
classFactory = new ClassFactoryImpl();
- proxyFactory = new PlasticProxyFactoryImpl(Thread.currentThread().getContextClassLoader(), null, new DummyLock());
+ proxyFactory = new PlasticProxyFactoryImpl(Thread.currentThread().getContextClassLoader(), null);
}
@AfterClass
@@ -227,7 +225,8 @@ public class DefaultModuleDefImplTest ex
new DefaultModuleDefImpl(ServiceIdConflictMethodModule.class, logger, proxyFactory);
unreachable();
- } catch (RuntimeException ex)
+ }
+ catch (RuntimeException ex)
{
assertMessageContains(
ex,
@@ -251,7 +250,8 @@ public class DefaultModuleDefImplTest ex
{
new DefaultModuleDefImpl(VoidBuilderMethodModule.class, logger, null);
unreachable();
- } catch (RuntimeException ex)
+ }
+ catch (RuntimeException ex)
{
assertEquals(ex.getMessage(), IOCMessages.buildMethodWrongReturnType(m));
}
@@ -272,7 +272,8 @@ public class DefaultModuleDefImplTest ex
{
new DefaultModuleDefImpl(BuilderMethodModule.class, logger, null);
unreachable();
- } catch (RuntimeException ex)
+ }
+ catch (RuntimeException ex)
{
assertEquals(ex.getMessage(), IOCMessages.buildMethodWrongReturnType(m));
}
@@ -298,7 +299,8 @@ public class DefaultModuleDefImplTest ex
{
new DefaultModuleDefImpl(moduleClass, logger, null);
unreachable();
- } catch (RuntimeException ex)
+ }
+ catch (RuntimeException ex)
{
assertEquals(ex.getMessage(), IOCMessages.decoratorMethodWrongReturnType(m));
}
@@ -377,7 +379,8 @@ public class DefaultModuleDefImplTest ex
{
new DefaultModuleDefImpl(moduleClass, logger, null);
unreachable();
- } catch (RuntimeException ex)
+ }
+ catch (RuntimeException ex)
{
assertEquals(
ex.getMessage(),
@@ -401,7 +404,8 @@ public class DefaultModuleDefImplTest ex
{
new DefaultModuleDefImpl(moduleClass, logger, null);
unreachable();
- } catch (RuntimeException ex)
+ }
+ catch (RuntimeException ex)
{
assertEquals(
ex.getMessage(),
@@ -460,7 +464,8 @@ public class DefaultModuleDefImplTest ex
{
new DefaultModuleDefImpl(UninstantiableAutobuildServiceModule.class, logger, null);
unreachable();
- } catch (RuntimeException ex)
+ }
+ catch (RuntimeException ex)
{
assertMessageContains(
ex,
@@ -481,7 +486,8 @@ public class DefaultModuleDefImplTest ex
{
new DefaultModuleDefImpl(NonStaticBindMethodModule.class, logger, proxyFactory);
unreachable();
- } catch (RuntimeException ex)
+ }
+ catch (RuntimeException ex)
{
assertMessageContains(ex,
"Method org.apache.tapestry5.ioc.internal.NonStaticBindMethodModule.bind(ServiceBinder)",
@@ -539,7 +545,8 @@ public class DefaultModuleDefImplTest ex
{
new DefaultModuleDefImpl(ExceptionInBindMethod.class, logger, proxyFactory);
unreachable();
- } catch (RuntimeException ex)
+ }
+ catch (RuntimeException ex)
{
assertTrue(ex.getMessage().matches(
"Error invoking service binder method org.apache.tapestry5.ioc.internal.ExceptionInBindMethod.bind\\(ServiceBinder\\) "