Author: hlship
Date: Thu Aug 7 16:38:09 2008
New Revision: 683747
URL: http://svn.apache.org/viewvc?rev=683747&view=rev
Log:
TAPESTRY-2561: Rapidly refreshing a page, even the same page, can cause a
deadlock related to class loading
Added:
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/InternalConstants.java
Modified:
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/internal/ModuleImpl.java
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/CtClassSourceImpl.java
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/services/MasterObjectProvider.java
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=683747&r1=683746&r2=683747&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 Aug 7 16:38:09 2008
@@ -18,6 +18,7 @@
import org.apache.tapestry5.internal.event.InvalidationEventHubImpl;
import org.apache.tapestry5.internal.events.UpdateListener;
import org.apache.tapestry5.internal.util.URLChangeTracker;
+import org.apache.tapestry5.ioc.internal.InternalConstants;
import org.apache.tapestry5.ioc.internal.services.ClassFactoryClassPool;
import org.apache.tapestry5.ioc.internal.services.ClassFactoryImpl;
import org.apache.tapestry5.ioc.internal.services.CtClassSource;
@@ -76,17 +77,24 @@
* Determines if the class name represents a component class from a
controlled package. If so,
* super.findClass() will load it and transform it. Returns null if
not in a controlled package, allowing the
* parent class loader to do the work.
- * <p/>
- * This method is synchronized to <em>attempt</em> to address
TAPESTRY-2468.
*
* @param className
* @return the loaded transformed Class, or null to force a load of
the class from the parent class loader
* @throws ClassNotFoundException
*/
@Override
- protected synchronized Class findClass(String className) throws
ClassNotFoundException
+ protected Class findClass(String className) throws
ClassNotFoundException
{
- if (inControlledPackage(className)) return
super.findClass(className);
+ if (inControlledPackage(className))
+ {
+ // TAPESTRY-2561: Prevent other threads from creating new
classes in either
+ // the component class loader or in the context class loader
(which is used for
+ // IoC proxies and the like). This is draconian, but the
deadlock issue remains.
+ synchronized (InternalConstants.GLOBAL_CLASS_CREATION_MUTEX)
+ {
+ return super.findClass(className);
+ }
+ }
// Returning null forces delegation to the parent class loader.
Added:
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/InternalConstants.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/InternalConstants.java?rev=683747&view=auto
==============================================================================
---
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/InternalConstants.java
(added)
+++
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/InternalConstants.java
Thu Aug 7 16:38:09 2008
@@ -0,0 +1,25 @@
+// Copyright 2008 The Apache Software Foundation
+//
+// 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 org.apache.tapestry5.ioc.internal;
+
+public class InternalConstants
+{
+ /**
+ * Mutex used to ensure that there are no thread conflicts when creating
new classes using Javassist.
+ * <p/>
+ * See TAPESTRY-2561.
+ */
+ public static final Object GLOBAL_CLASS_CREATION_MUTEX = new Object();
+}
Modified:
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java?rev=683747&r1=683746&r2=683747&view=diff
==============================================================================
---
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java
(original)
+++
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java
Thu Aug 7 16:38:09 2008
@@ -20,9 +20,8 @@
import org.apache.tapestry5.ioc.def.ModuleDef;
import org.apache.tapestry5.ioc.def.ServiceDef;
import org.apache.tapestry5.ioc.internal.services.JustInTimeObjectCreator;
-import static org.apache.tapestry5.ioc.internal.util.CollectionFactory.*;
-import static org.apache.tapestry5.ioc.internal.util.Defense.notBlank;
-import static org.apache.tapestry5.ioc.internal.util.Defense.notNull;
+import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
+import org.apache.tapestry5.ioc.internal.util.Defense;
import org.apache.tapestry5.ioc.internal.util.InternalUtils;
import org.apache.tapestry5.ioc.services.*;
import org.slf4j.Logger;
@@ -47,25 +46,18 @@
private final Logger logger;
- // Guarded by MUTEX
+ // Guarded by InternalConstants.GLOBAL_CLASS_CREATION_MUTEX
private Object moduleBuilder;
- /**
- * A single mutex, shared by all modules, that serializes creation of
services across all threads. This is a bit
- * draconian, but appears to be necessary. Fortunately, service creation
is a very tiny part of any individual
- * service's lifecycle.
- */
- private static final Object MUTEX = new Object();
-
// Set to true when invoking the module constructor. Used to
// detect endless loops caused by irresponsible dependencies in
- // the constructor. Guarded by MUTEX.
+ // the constructor. Guarded by
InternalConstants.GLOBAL_CLASS_CREATION_MUTEX.
private boolean insideConstructor;
/**
* Keyed on fully qualified service id; values are instantiated services
(proxies).
*/
- private final Map<String, Object> services = newCaseInsensitiveMap();
+ private final Map<String, Object> services =
CollectionFactory.newCaseInsensitiveMap();
public ModuleImpl(InternalRegistry registry, ServiceActivityTracker
tracker, ModuleDef moduleDef,
ClassFactory classFactory, Logger logger)
@@ -79,8 +71,8 @@
public <T> T getService(String serviceId, Class<T> serviceInterface)
{
- notBlank(serviceId, "serviceId");
- notNull(serviceInterface, "serviceInterface");
+ Defense.notBlank(serviceId, "serviceId");
+ Defense.notNull(serviceInterface, "serviceInterface");
// module may be null.
ServiceDef def = moduleDef.getServiceDef(serviceId);
@@ -107,7 +99,7 @@
public Set<DecoratorDef> findMatchingDecoratorDefs(ServiceDef serviceDef)
{
- Set<DecoratorDef> result = newSet();
+ Set<DecoratorDef> result = CollectionFactory.newSet();
for (DecoratorDef def : moduleDef.getDecoratorDefs())
{
@@ -127,9 +119,9 @@
@SuppressWarnings("unchecked")
public Collection<String> findServiceIdsForInterface(Class
serviceInterface)
{
- notNull(serviceInterface, "serviceInterface");
+ Defense.notNull(serviceInterface, "serviceInterface");
- Collection<String> result = newList();
+ Collection<String> result = CollectionFactory.newList();
for (String id : moduleDef.getServiceIds())
{
@@ -144,15 +136,15 @@
/**
* Locates the service proxy for a particular service (from the service
definition).
* <p/>
- * Access is synchronized via [EMAIL PROTECTED] #MUTEX}.
+ * Access is synchronized via [EMAIL PROTECTED]
InternalConstants#GLOBAL_CLASS_CREATION_MUTEX}.
*
* @param def defines the service
- * @param eagerLoadProxies TODO
+ * @param eagerLoadProxies collection into which proxies for eager loaded
services are added (or null)
* @return the service proxy
*/
private Object findOrCreate(ServiceDef def,
Collection<EagerLoadServiceProxy> eagerLoadProxies)
{
- synchronized (MUTEX)
+ synchronized (InternalConstants.GLOBAL_CLASS_CREATION_MUTEX)
{
String key = def.getServiceId();
@@ -170,7 +162,7 @@
public void collectEagerLoadServices(Collection<EagerLoadServiceProxy>
proxies)
{
- synchronized (MUTEX)
+ synchronized (InternalConstants.GLOBAL_CLASS_CREATION_MUTEX)
{
for (String serviceId : moduleDef.getServiceIds())
{
@@ -182,7 +174,8 @@
}
/**
- * Creates the service and updates the cache of created services. Access
is synchronized via [EMAIL PROTECTED] #MUTEX}.
+ * Creates the service and updates the cache of created services. Access
is synchronized via [EMAIL PROTECTED]
+ * InternalConstants#GLOBAL_CLASS_CREATION_MUTEX}.
*
* @param eagerLoadProxies a list into which any eager loaded proxies
should be added
*/
@@ -248,7 +241,7 @@
public Object getModuleBuilder()
{
- synchronized (MUTEX)
+ synchronized (InternalConstants.GLOBAL_CLASS_CREATION_MUTEX)
{
if (moduleBuilder == null) moduleBuilder =
instantiateModuleBuilder();
@@ -292,7 +285,7 @@
throw new
RuntimeException(IOCMessages.recursiveModuleConstructor(builderClass,
constructor));
ObjectLocator locator = new ObjectLocatorImpl(registry, this);
- Map<Class, Object> parameterDefaults = newMap();
+ Map<Class, Object> parameterDefaults = CollectionFactory.newMap();
parameterDefaults.put(Logger.class, logger);
parameterDefaults.put(ObjectLocator.class, locator);
@@ -342,11 +335,11 @@
ClassFab classFab = registry.newClass(serviceInterface);
- classFab.addField("_creator", Modifier.PRIVATE | Modifier.FINAL,
ObjectCreator.class);
- classFab.addField("_token", Modifier.PRIVATE | Modifier.FINAL,
ServiceProxyToken.class);
+ classFab.addField("creator", Modifier.PRIVATE | Modifier.FINAL,
ObjectCreator.class);
+ classFab.addField("token", Modifier.PRIVATE | Modifier.FINAL,
ServiceProxyToken.class);
- classFab.addConstructor(new Class[] { ObjectCreator.class,
ServiceProxyToken.class }, null,
- "{ _creator = $1; _token = $2; }");
+ classFab.addConstructor(new Class[]{ObjectCreator.class,
ServiceProxyToken.class}, null,
+ "{ creator = $1; token = $2; }");
// Make proxies serializable by writing the token to the stream.
@@ -355,19 +348,19 @@
// This is the "magic" signature that allows an object to substitute
some other
// object for itself.
MethodSignature writeReplaceSig = new MethodSignature(Object.class,
"writeReplace", null,
- new Class[] {
ObjectStreamException.class });
+ new
Class[]{ObjectStreamException.class});
- classFab.addMethod(Modifier.PRIVATE, writeReplaceSig, "return
_token;");
+ classFab.addMethod(Modifier.PRIVATE, writeReplaceSig, "return token;");
// Now delegate all the methods.
- String body = format("return (%s) _creator.createObject();",
serviceInterface.getName());
+ String body = format("return (%s) creator.createObject();",
serviceInterface.getName());
- MethodSignature sig = new MethodSignature(serviceInterface,
"_delegate", null, null);
+ MethodSignature sig = new MethodSignature(serviceInterface,
"delegate", null, null);
classFab.addMethod(Modifier.PRIVATE, sig, body);
- classFab.proxyMethodsToDelegate(serviceInterface, "_delegate()",
description);
+ classFab.proxyMethodsToDelegate(serviceInterface, "delegate()",
description);
Class proxyClass = classFab.createClass();
@@ -385,7 +378,7 @@
public Set<ContributionDef> getContributorDefsForService(String serviceId)
{
- Set<ContributionDef> result = newSet();
+ Set<ContributionDef> result = CollectionFactory.newSet();
for (ContributionDef def : moduleDef.getContributionDefs())
{
Modified:
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/CtClassSourceImpl.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/CtClassSourceImpl.java?rev=683747&r1=683746&r2=683747&view=diff
==============================================================================
---
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/CtClassSourceImpl.java
(original)
+++
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/CtClassSourceImpl.java
Thu Aug 7 16:38:09 2008
@@ -16,6 +16,7 @@
import javassist.CtClass;
import javassist.NotFoundException;
+import org.apache.tapestry5.ioc.internal.InternalConstants;
import org.apache.tapestry5.ioc.services.ClassFabUtils;
import java.security.ProtectionDomain;
@@ -87,17 +88,20 @@
{
if (WRITE_DIR != null) writeClass(ctClass);
- try
- {
- Class result = pool.toClass(ctClass, loader, domain);
-
- createdClassCount++;
-
- return result;
- }
- catch (Throwable ex)
+ synchronized (InternalConstants.GLOBAL_CLASS_CREATION_MUTEX)
{
- throw new
RuntimeException(ServiceMessages.unableToWriteClass(ctClass, ex), ex);
+ try
+ {
+ Class result = pool.toClass(ctClass, loader, domain);
+
+ createdClassCount++;
+
+ return result;
+ }
+ catch (Throwable ex)
+ {
+ throw new
RuntimeException(ServiceMessages.unableToWriteClass(ctClass, ex), ex);
+ }
}
}
Modified:
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/services/MasterObjectProvider.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/services/MasterObjectProvider.java?rev=683747&r1=683746&r2=683747&view=diff
==============================================================================
---
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/services/MasterObjectProvider.java
(original)
+++
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/services/MasterObjectProvider.java
Thu Aug 7 16:38:09 2008
@@ -19,7 +19,8 @@
import org.apache.tapestry5.ioc.ObjectProvider;
/**
- * Rolls up a number of [EMAIL PROTECTED] ObjectProvider}, but allows for the
case where no object may be provided.
+ * Rolls up a number of [EMAIL PROTECTED]
org.apache.tapestry5.ioc.ObjectProvider}, but allows for the case where no
object may be
+ * provided.
*/
public interface MasterObjectProvider
{
