Author: hlship
Date: Mon Sep 8 10:08:11 2008
New Revision: 693168
URL: http://svn.apache.org/viewvc?rev=693168&view=rev
Log:
TAPESTRY-2561: Rapidly refreshing a page, even the same page, can cause a
deadlock related to class loading
Modified:
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PagePoolImpl.java
tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml
tapestry/tapestry5/trunk/tapestry-core/src/test/conf/testng.xml
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Start.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/SerializationSupport.java
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/CtClassSourceImpl.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=693168&r1=693167&r2=693168&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
Mon Sep 8 10:08:11 2008
@@ -28,6 +28,7 @@
import org.slf4j.Logger;
import java.net.URL;
+import java.net.URLClassLoader;
import java.util.Map;
import java.util.Set;
@@ -72,21 +73,6 @@
super(parent, classPool);
}
-
- /**
- * Synchronizes on the parent class loader before continuing, which is
necessary to prevent thread deadlocks. Any classes
- * loaded, or transformed, by this class loader will do so with the
parent (context) class loader locked.
- * The required order is always that the context class loader be
locked, then the child class loader. Painful.
- */
- @Override
- protected Class loadClass(String name, boolean resolve) throws
ClassFormatError, ClassNotFoundException
- {
- synchronized (getParent())
- {
- return super.loadClass(name, resolve);
- }
- }
-
/**
* 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
@@ -101,13 +87,7 @@
{
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.
@@ -153,7 +133,13 @@
{
ClassFactoryClassPool classPool = new ClassFactoryClassPool(parent);
- loader = new PackageAwareLoader(parent, classPool);
+ // For TAPESTRY-2561, we're introducing a class loader between the
parent (i.e., the
+ // context class loader), and the component class loader, to try and
prevent the deadlocks
+ // that we've been seeing.
+
+ ClassLoader threadDeadlockBuffer = new URLClassLoader(new URL[0],
parent);
+
+ loader = new PackageAwareLoader(threadDeadlockBuffer, classPool);
ClassPath path = new LoaderClassPath(loader);
@@ -264,12 +250,20 @@
{
Instantiator result = classNameToInstantiator.get(className);
+ // Note: a race condition here can result in the temporary creation of
a duplicate instantiator.
+
if (result == null)
{
// Force the creation of the class (and the transformation of the
class).
findClass(className);
+ // Note: this is really a create, and in fact, will create a new
Class instance
+ // (it doesn't cache internally). This code is the only cache,
which is why
+ // the method is synchronized. We could use a ConcurrentBarrier,
but I suspect
+ // that the overhead of that is greater on a typical invocation
than
+ // the cost of the synchronization and the Map lookup.
+
result = transformer.createInstantiator(className);
classNameToInstantiator.put(className, result);
Modified:
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PagePoolImpl.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PagePoolImpl.java?rev=693168&r1=693167&r2=693168&view=diff
==============================================================================
---
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PagePoolImpl.java
(original)
+++
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PagePoolImpl.java
Mon Sep 8 10:08:11 2008
@@ -124,7 +124,6 @@
}
cache.release(page);
-
}
private synchronized PagePoolCache get(String pageName, Locale locale)
@@ -153,6 +152,10 @@
pool.clear();
}
+ /**
+ * On the periodic check for updates call, we clean up the caches,
discarding unsued and out of date page
+ * instances.
+ */
public synchronized void checkForUpdates()
{
for (PagePoolCache cache : pool.values())
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml?rev=693168&r1=693167&r2=693168&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml Mon Sep 8
10:08:11 2008
@@ -1,5 +1,4 @@
-<html t:type="Border"
- xmlns:t="http://tapestry.apache.org/schema/tapestry_5_0_0.xsd";>
+<html t:type="Border"
xmlns:t="http://tapestry.apache.org/schema/tapestry_5_0_0.xsd";>
<h1>Tapestry 5 Integration Application 1</h1>
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/conf/testng.xml
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/conf/testng.xml?rev=693168&r1=693167&r2=693168&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/conf/testng.xml (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/conf/testng.xml Mon Sep 8
10:08:11 2008
@@ -20,11 +20,6 @@
<suite name="Tapestry Core" annotations="1.5" verbose="2">
<test name="Integration Tests">
<packages>
- <package name="org.apache.tapestry5.integration"/>
- </packages>
- </test>
- <test name="Components">
- <packages>
<package name="org.apache.tapestry5.integration.pagelevel"/>
<package name="org.apache.tapestry5.corelib.base"/>
<package name="org.apache.tapestry5.corelib.components"/>
@@ -57,4 +52,9 @@
<package name="org.apache.tapestry5.internal.util"/>
</packages>
</test>
+ <test name="Integration Tests">
+ <packages>
+ <package name="org.apache.tapestry5.integration"/>
+ </packages>
+ </test>
</suite>
Modified:
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Start.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Start.java?rev=693168&r1=693167&r2=693168&view=diff
==============================================================================
---
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Start.java
(original)
+++
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Start.java
Mon Sep 8 10:08:11 2008
@@ -25,7 +25,7 @@
* Have to start somewhere!
*/
public class Start
-{
+{
public static class Item implements Comparable<Item>
{
private final String pageName;
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=693168&r1=693167&r2=693168&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
Mon Sep 8 10:08:11 2008
@@ -139,8 +139,8 @@
* @param eagerLoadProxies collection into which proxies for eager loaded
services are added (or null)
* @return the service proxy
*/
- private synchronized Object findOrCreate(final ServiceDef def,
- final
Collection<EagerLoadServiceProxy> eagerLoadProxies)
+ private Object findOrCreate(final ServiceDef def,
+ final Collection<EagerLoadServiceProxy>
eagerLoadProxies)
{
final String key = def.getServiceId();
Modified:
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/SerializationSupport.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/SerializationSupport.java?rev=693168&r1=693167&r2=693168&view=diff
==============================================================================
---
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/SerializationSupport.java
(original)
+++
tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/SerializationSupport.java
Mon Sep 8 10:08:11 2008
@@ -1,4 +1,4 @@
-// Copyright 2007 The Apache Software Foundation
+// Copyright 2007, 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.
@@ -40,6 +40,7 @@
providerRef = new WeakReference<ServiceProxyProvider>(proxyProvider);
}
+ // Only invoked from synchronized blocks
private static ServiceProxyProvider currentProvider()
{
return providerRef == null ? null : providerRef.get();
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=693168&r1=693167&r2=693168&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
Mon Sep 8 10:08:11 2008
@@ -87,23 +87,20 @@
{
if (WRITE_DIR != null) writeClass(ctClass);
- synchronized (loader)
+ try
{
- try
- {
- Class result = pool.toClass(ctClass, loader, domain);
+ Class result = pool.toClass(ctClass, loader, domain);
- synchronized (this)
- {
- createdClassCount++;
- }
-
- return result;
- }
- catch (Throwable ex)
+ synchronized (this)
{
- throw new
RuntimeException(ServiceMessages.unableToWriteClass(ctClass, ex), ex);
+ createdClassCount++;
}
+
+ return result;
+ }
+ catch (Throwable ex)
+ {
+ throw new
RuntimeException(ServiceMessages.unableToWriteClass(ctClass, ex), ex);
}
}
