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 2008/08/01 00:01:31 UTC

svn commit: r681513 [3/3] - in /tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry: ./ binding/ engine/ util/

Modified: tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/DefaultSpecificationSource.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/DefaultSpecificationSource.java?rev=681513&r1=681512&r2=681513&view=diff
==============================================================================
--- tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/DefaultSpecificationSource.java (original)
+++ tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/DefaultSpecificationSource.java Thu Jul 31 15:01:31 2008
@@ -15,7 +15,6 @@
 package org.apache.tapestry.engine;
 
 import edu.emory.mathcs.backport.java.util.concurrent.ConcurrentHashMap;
-import edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantLock;
 import org.apache.commons.lang.builder.ToStringBuilder;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -27,6 +26,7 @@
 import org.apache.tapestry.spec.ILibrarySpecification;
 import org.apache.tapestry.spec.LibrarySpecification;
 import org.apache.tapestry.util.IRenderDescription;
+import org.apache.tapestry.util.ResourceLockManager;
 import org.apache.tapestry.util.pool.Pool;
 import org.apache.tapestry.util.xml.DocumentParseException;
 
@@ -35,31 +35,26 @@
 import java.util.Set;
 
 /**
- *  Default implementation of {@link ISpecificationSource} that
- *  expects to use the normal class loader to locate component
- *  specifications from within the classpath.
- *
+ * Default implementation of {@link ISpecificationSource} that expects to use the normal class loader to locate
+ * component specifications from within the classpath.
+ * <p/>
  * <p>Caches specifications in memory forever, or until {@link #reset()} is invoked.
- *
- * <p>An instance of this class acts like a singleton and is shared by multiple sessions,
- * so it must be threadsafe.
+ * <p/>
+ * <p>An instance of this class acts like a singleton and is shared by multiple sessions, so it must be threadsafe.
  *
  * @author Howard Lewis Ship
  * @version $Id$
- *
- **/
+ */
 
 public class DefaultSpecificationSource implements ISpecificationSource, IRenderDescription
 {
     private static final Log LOG = LogFactory.getLog(DefaultSpecificationSource.class);
 
     /**
-     *  Key used to get and store {@link SpecificationParser} instances
-     *  from the Pool.
+     * Key used to get and store {@link SpecificationParser} instances from the Pool.
      *
-     *  @since 3.0
-     *
-     **/
+     * @since 3.0
+     */
 
     private static final String PARSER_POOL_KEY = "org.apache.tapestry.SpecificationParser";
 
@@ -70,60 +65,42 @@
     private INamespace _frameworkNamespace;
 
     /**
-     *  Contains previously parsed component specifications.
-     *
-     **/
+     * Contains previously parsed component specifications.
+     */
 
-    private Map _componentCache = new ConcurrentHashMap();
+    private final Map _componentCache = new ConcurrentHashMap();
 
     /**
-     *  Contains previously parsed page specifications.
+     * Contains previously parsed page specifications.
      *
-     *  @since 2.2
-     *
-     **/
+     * @since 2.2
+     */
 
-    private Map _pageCache = new ConcurrentHashMap();
+    private final Map _pageCache = new ConcurrentHashMap();
 
     /**
-     *  Contains previously parsed library specifications, keyed
-     *  on specification resource path.
+     * Contains previously parsed library specifications, keyed on specification resource path.
      *
-     *  @since 2.2
-     *
-     **/
-
-    private Map _libraryCache = new ConcurrentHashMap();
+     * @since 2.2
+     */
 
-    /**
-     *  Contains {@link INamespace} instances, keyed on id (which will
-     *  be null for the application specification).
-     *
-     **/
+    private final Map _libraryCache = new ConcurrentHashMap();
 
-    private Map _namespaceCache = new ConcurrentHashMap();
 
     /**
      * Used to synchronize concurrent operations on specific resources.
      */
-    private ConcurrentHashMap _lockCache = new ConcurrentHashMap();
+    private final ResourceLockManager _resourceLockManager = new ResourceLockManager();
 
     /**
-     *  Reference to the shared {@link org.apache.tapestry.util.pool.Pool}.
-     *
-     *  @see org.apache.tapestry.IEngine#getPool()
+     * Reference to the shared {@link org.apache.tapestry.util.pool.Pool}.
      *
-     *  @since 3.0
-     *
-     **/
+     * @see org.apache.tapestry.IEngine#getPool()
+     * @since 3.0
+     */
 
     private Pool _pool;
 
-    /**
-     * Used to synchronize global member access.
-     */
-    private ReentrantLock _monitor = new ReentrantLock();
-
     public DefaultSpecificationSource(
             IResourceResolver resolver,
             IApplicationSpecification specification,
@@ -135,28 +112,18 @@
     }
 
     /**
-     *  Clears the specification cache.  This is used during debugging.
-     *
-     **/
+     * Clears the specification cache.  This is used during debugging.
+     */
 
-    public void reset()
+    public synchronized void reset()
     {
-        _monitor.lock();
+        _componentCache.clear();
+        _pageCache.clear();
+        _libraryCache.clear();
+        _resourceLockManager.clear();
 
-        try
-        {
-            _componentCache.clear();
-            _pageCache.clear();
-            _libraryCache.clear();
-            _namespaceCache.clear();
-            _lockCache.clear();
-
-            _applicationNamespace = null;
-            _frameworkNamespace = null;
-        } finally
-        {
-            _monitor.unlock();
-        }
+        _applicationNamespace = null;
+        _frameworkNamespace = null;
     }
 
     protected IComponentSpecification parseSpecification(
@@ -224,7 +191,9 @@
         return builder.toString();
     }
 
-    /** @since 1.0.6 **/
+    /**
+     * @since 1.0.6 *
+     */
 
     public void renderDescription(IMarkupWriter writer)
     {
@@ -273,12 +242,11 @@
     }
 
     /**
-     *  Gets a component specification.
-     *
-     *  @param resourceLocation the complete resource path to the specification.
-     *  @throws ApplicationRuntimeException if the specification cannot be obtained.
+     * Gets a component specification.
      *
-     **/
+     * @param resourceLocation the complete resource path to the specification.
+     * @throws ApplicationRuntimeException if the specification cannot be obtained.
+     */
 
     public IComponentSpecification getComponentSpecification(IResourceLocation resourceLocation)
     {
@@ -288,9 +256,7 @@
         if (result != null)
             return result;
 
-        ReentrantLock lock = getLock(resourceLocation);
-
-        lock.lock();
+        _resourceLockManager.lock(resourceLocation);
 
         try
         {
@@ -302,9 +268,11 @@
             result = parseSpecification(resourceLocation, false);
 
             _componentCache.put(resourceLocation, result);
-        } finally
+        }
+        finally
         {
-            lock.unlock();
+            _resourceLockManager.unlock(resourceLocation);
+
         }
 
         return result;
@@ -317,23 +285,27 @@
         if (result != null)
             return result;
 
-        ReentrantLock lock = getLock(resourceLocation);
-
-        lock.lock();
+        _resourceLockManager.lock(resourceLocation);
 
         try
         {
+            // In a race condition, one thread may be parsing the specification while others block.
+            // The specification is in the cache once they un-block.
+
             result = (IComponentSpecification) _pageCache.get(resourceLocation);
 
+
             if (result != null)
                 return result;
 
             result = parseSpecification(resourceLocation, true);
 
             _pageCache.put(resourceLocation, result);
-        } finally
+        }
+        finally
         {
-            lock.unlock();
+            _resourceLockManager.unlock(resourceLocation);
+
         }
 
         return result;
@@ -346,12 +318,13 @@
         if (result != null)
             return result;
 
-        ReentrantLock lock = getLock(resourceLocation);
-
-        lock.lock();
+        _resourceLockManager.lock(resourceLocation);
 
         try
         {
+            // In a race condition, one thread may be parsing the specification while others block.
+            // The specification is in the cache once they un-block.
+
             result = (LibrarySpecification) _libraryCache.get(resourceLocation);
 
             if (result != null)
@@ -359,15 +332,19 @@
 
             result = parseLibrarySpecification(resourceLocation);
             _libraryCache.put(resourceLocation, result);
-        } finally
+        }
+        finally
         {
-            lock.unlock();
+            _resourceLockManager.unlock(resourceLocation);
+
         }
 
         return result;
     }
 
-    /** @since 2.2 **/
+    /**
+     * @since 2.2 *
+     */
 
     protected SpecificationParser getParser()
     {
@@ -379,59 +356,35 @@
         return result;
     }
 
-    /** @since 3.0 **/
+    /**
+     * @since 3.0 *
+     */
 
     protected void discardParser(SpecificationParser parser)
     {
         _pool.store(PARSER_POOL_KEY, parser);
     }
 
-    public INamespace getApplicationNamespace()
+    public synchronized INamespace getApplicationNamespace()
     {
-        _monitor.lock();
+        if (_applicationNamespace == null)
+            _applicationNamespace = new Namespace(null, null, _specification, this);
 
-        try
-        {
-            if (_applicationNamespace == null)
-                _applicationNamespace = new Namespace(null, null, _specification, this);
-
-            return _applicationNamespace;
-
-        } finally
-        {
-            _monitor.unlock();
-        }
+        return _applicationNamespace;
     }
 
-    public INamespace getFrameworkNamespace()
+    public synchronized INamespace getFrameworkNamespace()
     {
-        _monitor.lock();
-
-        try
+        if (_frameworkNamespace == null)
         {
-            if (_frameworkNamespace == null)
-            {
-                IResourceLocation frameworkLocation =
-                        new ClasspathResourceLocation(_resolver, "/org/apache/tapestry/Framework.library");
+            IResourceLocation frameworkLocation =
+                    new ClasspathResourceLocation(_resolver, "/org/apache/tapestry/Framework.library");
 
-                ILibrarySpecification ls = getLibrarySpecification(frameworkLocation);
+            ILibrarySpecification ls = getLibrarySpecification(frameworkLocation);
 
-                _frameworkNamespace = new Namespace(INamespace.FRAMEWORK_NAMESPACE, null, ls, this);
-            }
-
-            return _frameworkNamespace;
-
-        } finally
-        {
-            _monitor.unlock();
+            _frameworkNamespace = new Namespace(INamespace.FRAMEWORK_NAMESPACE, null, ls, this);
         }
-    }
-
-    private ReentrantLock getLock(Object key)
-    {
-        _lockCache.putIfAbsent(key, new ReentrantLock());
 
-        return (ReentrantLock) _lockCache.get(key);
+        return _frameworkNamespace;
     }
-
 }
\ No newline at end of file

Modified: tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/DefaultTemplateSource.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/DefaultTemplateSource.java?rev=681513&r1=681512&r2=681513&view=diff
==============================================================================
--- tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/DefaultTemplateSource.java (original)
+++ tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/DefaultTemplateSource.java Thu Jul 31 15:01:31 2008
@@ -15,7 +15,6 @@
 package org.apache.tapestry.engine;
 
 import edu.emory.mathcs.backport.java.util.concurrent.ConcurrentHashMap;
-import edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantLock;
 import org.apache.commons.lang.builder.ToStringBuilder;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -35,22 +34,18 @@
 import java.util.Map;
 
 /**
- *  Default implementation of {@link ITemplateSource}.  Templates, once parsed,
- *  stay in memory until explicitly cleared.
+ * Default implementation of {@link ITemplateSource}.  Templates, once parsed, stay in memory until explicitly cleared.
+ * <p/>
+ * <p>An instance of this class acts as a singleton shared by all sessions, so it must be threadsafe.
  *
- *  <p>An instance of this class acts as a singleton shared by all sessions, so it
- *  must be threadsafe.
- *
- *  @author Howard Lewis Ship
- *  @version $Id$
- *
- **/
+ * @author Howard Lewis Ship
+ * @version $Id$
+ */
 
 public class DefaultTemplateSource implements ITemplateSource, IRenderDescription
 {
     private static final Log LOG = LogFactory.getLog(DefaultTemplateSource.class);
 
-
     // The name of the component/application/etc property that will be used to
     // determine the encoding to use when loading the template
 
@@ -60,20 +55,18 @@
     // specification resource path and locale (local may be null), value
     // is the ComponentTemplate.
 
-    private Map _cache = new ConcurrentHashMap();
+    private final Map _cache = new ConcurrentHashMap();
 
     // Previously read templates; key is the IResourceLocation, value
     // is the ComponentTemplate.
 
-    private Map _templates = new ConcurrentHashMap();
+    private final Map _templates = new ConcurrentHashMap();
 
-    // Used to synchronize access to specific templates
-    private ConcurrentHashMap _lockCache = new ConcurrentHashMap();
+    private final ResourceLockManager _resourceLockManager = new ResourceLockManager();
 
     /**
-     *  Number of tokens (each template contains multiple tokens).
-     *
-     **/
+     * Number of tokens (each template contains multiple tokens).
+     */
 
     private int _tokenCount;
 
@@ -81,34 +74,36 @@
 
     private TemplateParser _parser;
 
-    /** @since 2.2 **/
+    /**
+     * @since 2.2 *
+     */
 
     private IResourceLocation _applicationRootLocation;
 
-    /** @since 3.0 **/
+    /**
+     * @since 3.0 *
+     */
 
     private ITemplateSourceDelegate _delegate;
 
     /**
-     *  Clears the template cache.  This is used during debugging.
-     *
-     **/
+     * Clears the template cache.  This is used during debugging.
+     */
 
     public void reset()
     {
         _cache.clear();
         _templates.clear();
-        _lockCache.clear();
+        _resourceLockManager.clear();
 
         _tokenCount = 0;
     }
 
     /**
-     *  Reads the template for the component.
-     *
-     *  <p>Returns null if the template can't be found.
-     *
-     **/
+     * Reads the template for the component.
+     * <p/>
+     * <p>Returns null if the template can't be found.
+     */
 
     public ComponentTemplate getTemplate(IRequestCycle cycle, IComponent component)
     {
@@ -117,17 +112,14 @@
 
         Locale locale = component.getPage().getLocale();
 
-        Object key = new MultiKey(new Object[] { specificationLocation, locale }, false);
+        Object key = new MultiKey(new Object[]{specificationLocation, locale}, false);
 
         ComponentTemplate result = searchCache(key);
         if (result != null)
             return result;
 
-        _lockCache.putIfAbsent(key, new ReentrantLock());
-
-        ReentrantLock lock = (ReentrantLock)_lockCache.get(key);
+        _resourceLockManager.lock(key);
 
-        lock.lock();
         try
         {
             result = searchCache(key);
@@ -145,8 +137,8 @@
 
                 String stringKey =
                         component.getSpecification().isPageSpecification()
-                                ? "DefaultTemplateSource.no-template-for-page"
-                                : "DefaultTemplateSource.no-template-for-component";
+                        ? "DefaultTemplateSource.no-template-for-page"
+                        : "DefaultTemplateSource.no-template-for-component";
 
                 throw new ApplicationRuntimeException(
                         Tapestry.format(stringKey, component.getExtendedId(), locale),
@@ -156,11 +148,14 @@
             }
 
             saveToCache(key, result);
-        } finally
+
+            return result;
+        }
+        finally
         {
-            lock.unlock();
+            _resourceLockManager.unlock(key);
         }
-        return result;
+
     }
 
     private ComponentTemplate searchCache(Object key)
@@ -198,17 +193,12 @@
     }
 
     /**
-     *  Finds the template for the given component, using the following rules:
-     *  <ul>
-     *  <li>If the component has a $template asset, use that
-     *  <li>Look for a template in the same folder as the component
-     *  <li>If a page in the application namespace, search in the application root
-     *  <li>Fail!
-     *  </ul>
-     *
-     *  @return the template, or null if not found
+     * Finds the template for the given component, using the following rules: <ul> <li>If the component has a $template
+     * asset, use that <li>Look for a template in the same folder as the component <li>If a page in the application
+     * namespace, search in the application root <li>Fail! </ul>
      *
-     **/
+     * @return the template, or null if not found
+     */
 
     private ComponentTemplate findTemplate(
             IRequestCycle cycle,
@@ -229,8 +219,8 @@
                 findStandardTemplate(cycle, location, component, templateBaseName, locale);
 
         if (result == null
-            && component.getSpecification().isPageSpecification()
-            && component.getNamespace().isApplicationNamespace())
+                && component.getSpecification().isPageSpecification()
+                && component.getNamespace().isApplicationNamespace())
             result = findPageTemplateInApplicationRoot(cycle, component, templateBaseName, locale);
 
         return result;
@@ -259,9 +249,8 @@
     }
 
     /**
-     *  Reads an asset to get the template.
-     *
-     **/
+     * Reads an asset to get the template.
+     */
 
     private ComponentTemplate readTemplateFromAsset(
             IRequestCycle cycle,
@@ -293,13 +282,11 @@
     }
 
     /**
-     *  Search for the template corresponding to the resource and the locale.
-     *  This may be in the template map already, or may involve reading and
-     *  parsing the template.
-     *
-     *  @return the template, or null if not found.
+     * Search for the template corresponding to the resource and the locale. This may be in the template map already, or
+     * may involve reading and parsing the template.
      *
-     **/
+     * @return the template, or null if not found.
+     */
 
     private ComponentTemplate findStandardTemplate(
             IRequestCycle cycle,
@@ -311,9 +298,9 @@
         if (LOG.isDebugEnabled())
             LOG.debug(
                     "Searching for localized version of template for "
-                    + location
-                    + " in locale "
-                    + locale.getDisplayName());
+                            + location
+                            + " in locale "
+                            + locale.getDisplayName());
 
         IResourceLocation baseTemplateLocation = location.getRelativeLocation(templateBaseName);
 
@@ -327,11 +314,9 @@
     }
 
     /**
-     *  Returns a previously parsed template at the specified location (which must already
-     *  be localized).  If not already in the template Map, then the
-     *  location is parsed and stored into the templates Map, then returned.
-     *
-     **/
+     * Returns a previously parsed template at the specified location (which must already be localized).  If not already
+     * in the template Map, then the location is parsed and stored into the templates Map, then returned.
+     */
 
     private ComponentTemplate getOrParseTemplate(
             IRequestCycle cycle,
@@ -354,12 +339,9 @@
     }
 
     /**
-     *  Reads the template for the given resource; returns null if the
-     *  resource doesn't exist.  Note that this method is only invoked
-     *  from a synchronized block, so there shouldn't be threading
-     *  issues here.
-     *
-     **/
+     * Reads the template for the given resource; returns null if the resource doesn't exist.  Note that this method is
+     * only invoked from a synchronized block, so there shouldn't be threading issues here.
+     */
 
     private ComponentTemplate parseTemplate(
             IRequestCycle cycle,
@@ -376,12 +358,9 @@
     }
 
     /**
-     *  This method is currently synchronized, because
-     *  {@link TemplateParser} is not threadsafe.  Another good candidate
-     *  for a pooling mechanism, especially because parsing a template
-     *  may take a while.
-     *
-     **/
+     * This method is currently synchronized, because {@link TemplateParser} is not threadsafe.  Another good candidate
+     * for a pooling mechanism, especially because parsing a template may take a while.
+     */
 
     private synchronized ComponentTemplate constructTemplateInstance(
             IRequestCycle cycle,
@@ -416,10 +395,8 @@
     }
 
     /**
-     *  Reads the template, given the complete path to the
-     *  resource.  Returns null if the resource doesn't exist.
-     *
-     **/
+     * Reads the template, given the complete path to the resource.  Returns null if the resource doesn't exist.
+     */
 
     private char[] readTemplate(IResourceLocation location, String encoding)
     {
@@ -461,9 +438,8 @@
     }
 
     /**
-     *  Reads a Stream into memory as an array of characters.
-     *
-     **/
+     * Reads a Stream into memory as an array of characters.
+     */
 
     private char[] readTemplateStream(InputStream stream, String encoding) throws IOException
     {
@@ -520,11 +496,10 @@
     }
 
     /**
-     *  Checks for the {@link Tapestry#TEMPLATE_EXTENSION_PROPERTY} in the component's
-     *  specification, then in the component's namespace's specification.  Returns
-     *  {@link Tapestry#DEFAULT_TEMPLATE_EXTENSION} if not otherwise overriden.
-     *
-     **/
+     * Checks for the {@link Tapestry#TEMPLATE_EXTENSION_PROPERTY} in the component's specification, then in the
+     * component's namespace's specification.  Returns {@link Tapestry#DEFAULT_TEMPLATE_EXTENSION} if not otherwise
+     * overriden.
+     */
 
     private String getTemplateExtension(IComponent component)
     {
@@ -544,7 +519,9 @@
         return Tapestry.DEFAULT_TEMPLATE_EXTENSION;
     }
 
-    /** @since 1.0.6 **/
+    /**
+     * @since 1.0.6 *
+     */
 
     public synchronized void renderDescription(IMarkupWriter writer)
     {

Modified: tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/ExpressionCacheImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/ExpressionCacheImpl.java?rev=681513&r1=681512&r2=681513&view=diff
==============================================================================
--- tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/ExpressionCacheImpl.java (original)
+++ tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/ExpressionCacheImpl.java Thu Jul 31 15:01:31 2008
@@ -14,7 +14,6 @@
 
 package org.apache.tapestry.engine;
 
-import edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantLock;
 import ognl.ClassCacheInspector;
 import ognl.Node;
 import ognl.OgnlRuntime;
@@ -27,9 +26,8 @@
  * @author Howard M. Lewis Ship
  * @since 4.0
  */
-public class ExpressionCacheImpl implements ExpressionCache, ClassCacheInspector {
-
-    private final ReentrantLock _lock = new ReentrantLock();
+public class ExpressionCacheImpl implements ExpressionCache, ClassCacheInspector
+{
 
     private final Map _compiledExpressionCache = new HashMap();
 
@@ -50,77 +48,53 @@
         }
     }
 
-    public void reset()
+    public synchronized void reset()
     {
-        try
-        {
-            _lock.lock();
-
-            _compiledExpressionCache.clear();
-            _expressionCache.clear();
-        } finally
-        {
-            _lock.unlock();
-        }
+        _compiledExpressionCache.clear();
+        _expressionCache.clear();
     }
 
     public boolean shouldCache(Class type)
     {
         if (!_cachingDisabled || type == null
-            || AbstractComponent.class.isAssignableFrom(type))
+                || AbstractComponent.class.isAssignableFrom(type))
             return false;
 
         return true;
     }
 
-    public Object get(Object target, String expression)
+    public synchronized Object get(Object target, String expression)
     {
-        try
+        Map cached = (Map) _compiledExpressionCache.get(target.getClass());
+        if (cached == null)
         {
-            _lock.lock();
-
-            Map cached = (Map)_compiledExpressionCache.get(target.getClass());
-            if (cached == null)
-            {
-                return _expressionCache.get(expression);
-            } else
-            {
-                return cached.get(expression);
-            }
-
-        } finally
+            return _expressionCache.get(expression);
+        }
+        else
         {
-            _lock.unlock();
+            return cached.get(expression);
         }
     }
 
-    public void cache(Object target, String expression, Node node)
+    public synchronized void cache(Object target, String expression, Node node)
     {
-        try
+        if (node.getAccessor() != null)
         {
-            _lock.lock();
-
-            if (node.getAccessor() != null)
-            {
-                Map cached = (Map)_compiledExpressionCache.get(target.getClass());
-
-                if (cached == null)
-                {
-                    cached = new HashMap();
-                    _compiledExpressionCache.put(target.getClass(), cached);
-                }
+            Map cached = (Map) _compiledExpressionCache.get(target.getClass());
 
-                cached.put(expression, node);
-
-                _expressionCache.remove(target.getClass());
-            } else
+            if (cached == null)
             {
-                _expressionCache.put(expression, node);
+                cached = new HashMap();
+                _compiledExpressionCache.put(target.getClass(), cached);
             }
 
-        } finally
+            cached.put(expression, node);
+
+            _expressionCache.remove(target.getClass());
+        }
+        else
         {
-            _lock.unlock();
+            _expressionCache.put(expression, node);
         }
     }
 }

Modified: tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/ExpressionEvaluatorImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/ExpressionEvaluatorImpl.java?rev=681513&r1=681512&r2=681513&view=diff
==============================================================================
--- tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/ExpressionEvaluatorImpl.java (original)
+++ tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/ExpressionEvaluatorImpl.java Thu Jul 31 15:01:31 2008
@@ -29,7 +29,8 @@
 /**
  * @since 4.0
  */
-public class ExpressionEvaluatorImpl implements ExpressionEvaluator {
+public class ExpressionEvaluatorImpl implements ExpressionEvaluator
+{
 
     private static final long POOL_MIN_IDLE_TIME = 1000 * 60 * 50;
 
@@ -72,7 +73,8 @@
     void initializeService()
     {
         if (_applicationSpecification.checkExtension(Tapestry.OGNL_TYPE_CONVERTER))
-            _typeConverter = (TypeConverter) _applicationSpecification.getExtension(Tapestry.OGNL_TYPE_CONVERTER, TypeConverter.class);
+            _typeConverter = (TypeConverter) _applicationSpecification.getExtension(Tapestry.OGNL_TYPE_CONVERTER,
+                                                                                    TypeConverter.class);
 
         _defaultContext = Ognl.createDefaultContext(null, _ognlResolver, _typeConverter);
 
@@ -88,17 +90,19 @@
 
     public Node parse(Object target, String expression)
     {
-        Node node = (Node)_expressionCache.get(target, expression);
+        Node node = (Node) _expressionCache.get(target, expression);
 
         if (node == null)
         {
             try
             {
-                node = (Node)Ognl.parseExpression(expression);
+                node = (Node) Ognl.parseExpression(expression);
                 _expressionCache.cache(target, expression, node);
-            } catch (OgnlException ex)
+            }
+            catch (OgnlException ex)
             {
-                throw new ApplicationRuntimeException(Tapestry.format("unable-to-read-expression", expression, target, ex), target, null, ex);
+                throw new ApplicationRuntimeException(
+                        Tapestry.format("unable-to-read-expression", expression, target, ex), target, null, ex);
             }
         }
 
@@ -120,33 +124,57 @@
         OgnlContext context = null;
         try
         {
-            context = (OgnlContext)_contextPool.borrowObject();
+            context = (OgnlContext) _contextPool.borrowObject();
             context.setRoot(target);
-            
+
             return Ognl.getValue(expression, context, target);
         }
         catch (Exception ex)
         {
-            throw new ApplicationRuntimeException(Tapestry.format("unable-to-read-expression", expression, target, ex), target, null, ex);
-        } finally {
-            try { if (context != null) _contextPool.returnObject(context); } catch (Exception e) {}
+            throw new ApplicationRuntimeException(Tapestry.format("unable-to-read-expression", expression, target, ex),
+                                                  target, null, ex);
+        }
+        finally
+        {
+            discardContext(context);
+        }
+    }
+
+    /**
+     * Invoked from various finally blocks to discard a OgnlContext used during an operation.
+     */
+    private void discardContext(OgnlContext context)
+    {
+        if (context == null) return;
+
+        try
+        {
+            _contextPool.returnObject(context);
+        }
+        catch (Exception ex)
+        {
+            // Ignored.  Should be logged?
         }
     }
 
     public Object read(Object target, ExpressionAccessor expression)
     {
         OgnlContext context = null;
+
         try
         {
-            context = (OgnlContext)_contextPool.borrowObject();
+            context = (OgnlContext) _contextPool.borrowObject();
 
             return expression.get(context, target);
         }
         catch (Exception ex)
         {
-            throw new ApplicationRuntimeException(Tapestry.format("unable-to-read-expression", expression, target, ex), target, null, ex);
-        } finally {
-            try { if (context != null) _contextPool.returnObject(context); } catch (Exception e) {}
+            throw new ApplicationRuntimeException(Tapestry.format("unable-to-read-expression", expression, target, ex),
+                                                  target, null, ex);
+        }
+        finally
+        {
+            discardContext(context);
         }
     }
 
@@ -160,7 +188,7 @@
         OgnlContext context = null;
         try
         {
-            context = (OgnlContext)_contextPool.borrowObject();
+            context = (OgnlContext) _contextPool.borrowObject();
 
             // set up context
             context.setRoot(target);
@@ -170,10 +198,12 @@
         catch (Exception ex)
         {
             throw new ApplicationRuntimeException(Tapestry.format("unable-to-write-expression",
-                                                                  new Object[] {expression, target, value, ex}),
+                                                                  new Object[]{expression, target, value, ex}),
                                                   target, null, ex);
-        } finally {
-            try { if (context != null) _contextPool.returnObject(context); } catch (Exception e) {}
+        }
+        finally
+        {
+            discardContext(context);
         }
     }
 
@@ -182,17 +212,19 @@
         OgnlContext context = null;
         try
         {
-            context = (OgnlContext)_contextPool.borrowObject();
+            context = (OgnlContext) _contextPool.borrowObject();
 
             Ognl.setValue(expression, context, target, value);
         }
         catch (Exception ex)
         {
             throw new ApplicationRuntimeException(Tapestry.format("unable-to-write-expression",
-                                                                  new Object[] {expression, target, value, ex}),
+                                                                  new Object[]{expression, target, value, ex}),
                                                   target, null, ex);
-        } finally {
-            try { if (context != null) _contextPool.returnObject(context); } catch (Exception e) {}
+        }
+        finally
+        {
+            discardContext(context);
         }
     }
 
@@ -218,10 +250,10 @@
     public void compileExpression(Object target, Node node, String expression)
     {
         OgnlContext context = null;
-        
+
         try
         {
-            context = (OgnlContext)_contextPool.borrowObject();
+            context = (OgnlContext) _contextPool.borrowObject();
 
             // set up context
             context.setRoot(target);
@@ -230,11 +262,15 @@
 
             _expressionCache.cache(target, expression, node);
 
-        } catch (Exception ex)
+        }
+        catch (Exception ex)
         {
-            throw new ApplicationRuntimeException(Tapestry.format("unable-to-read-expression", expression, target, ex), target, null, ex);
-        } finally {
-            try { if (context != null) _contextPool.returnObject(context); } catch (Exception e) {}
+            throw new ApplicationRuntimeException(Tapestry.format("unable-to-read-expression", expression, target, ex),
+                                                  target, null, ex);
+        }
+        finally
+        {
+            discardContext(context);
         }
     }
 
@@ -246,7 +282,9 @@
 
             OgnlRuntime.clearCache();
             Introspector.flushCaches();
-        } catch (Exception et) {
+        }
+        catch (Exception et)
+        {
             // ignore
         }
     }

Added: tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/util/ResourceLockManager.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/util/ResourceLockManager.java?rev=681513&view=auto
==============================================================================
--- tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/util/ResourceLockManager.java (added)
+++ tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/util/ResourceLockManager.java Thu Jul 31 15:01:31 2008
@@ -0,0 +1,57 @@
+//  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.tapestry.util;
+
+import edu.emory.mathcs.backport.java.util.concurrent.ConcurrentHashMap;
+import edu.emory.mathcs.backport.java.util.concurrent.ConcurrentMap;
+import edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantLock;
+
+/**
+ * Manages {@link edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantLock}s keyed on some arbitrary object.
+ */
+public class ResourceLockManager
+{
+    /**
+     * Map from some object (which should be a proper key) to a ReentrantLock.
+     */
+    private final ConcurrentMap resourceToLock = new ConcurrentHashMap();
+
+    public void clear()
+    {
+        resourceToLock.clear();
+    }
+
+    private ReentrantLock get(Object object)
+    {
+        if (!resourceToLock.containsKey(object))
+        {
+            // Watch out for a race condition where two threads may put conflicting locks
+            // for the name resource.
+            resourceToLock.putIfAbsent(object, new ReentrantLock());
+        }
+
+        return (ReentrantLock) resourceToLock.get(object);
+    }
+
+    public void lock(Object object)
+    {
+        get(object).lock();
+    }
+
+    public void unlock(Object object)
+    {
+        get(object).unlock();
+    }
+}