You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by ja...@apache.org on 2010/10/16 21:40:14 UTC

svn commit: r1023364 - in /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces: el/convert/VariableResolverToELResolver.java view/facelets/tag/MetaRulesetImpl.java

Author: jakobk
Date: Sat Oct 16 19:40:14 2010
New Revision: 1023364

URL: http://svn.apache.org/viewvc?rev=1023364&view=rev
Log:
MYFACES-2942 Memory Leak in MyFaces 2.0.1 probably as well in 2.0.2 (WeakHashMap fix in MetaRulesetImpl + ThreadLocal fix in VariableResolverToELResolver)

Modified:
    myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/el/convert/VariableResolverToELResolver.java
    myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/view/facelets/tag/MetaRulesetImpl.java

Modified: myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/el/convert/VariableResolverToELResolver.java
URL: http://svn.apache.org/viewvc/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/el/convert/VariableResolverToELResolver.java?rev=1023364&r1=1023363&r2=1023364&view=diff
==============================================================================
--- myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/el/convert/VariableResolverToELResolver.java (original)
+++ myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/el/convert/VariableResolverToELResolver.java Sat Oct 16 19:40:14 2010
@@ -26,7 +26,6 @@ import javax.el.PropertyNotWritableExcep
 import javax.faces.context.FacesContext;
 import javax.faces.el.EvaluationException;
 import javax.faces.el.VariableResolver;
-
 import java.beans.FeatureDescriptor;
 import java.util.Collection;
 import java.util.HashSet;
@@ -44,13 +43,32 @@ public final class VariableResolverToELR
 {
 
     // holds a flag to check if this instance is already called in current thread 
-    private static final ThreadLocal<Collection<String>> propertyGuard = new ThreadLocal<Collection<String>>() {
-        @Override
-        protected Collection<String> initialValue()
+    private static final ThreadLocal<Collection<String>> propertyGuardThreadLocal
+            = new ThreadLocal<Collection<String>>();
+
+    /**
+     * Gets the Collection<String> value of the propertyGuardThreadLocal.
+     * If the value from the ThreadLocal ist null, a new Collection<String>
+     * will be created.
+     *
+     * NOTE that we should not accomplish this by setting an initialValue on the
+     * ThreadLocal, because this will automatically be set on the ThreadLocalMap
+     * and thus can propably cause a memory leak.
+     *
+     * @return
+     */
+    private static Collection<String> getPropertyGuard()
+    {
+        Collection<String> propertyGuard = propertyGuardThreadLocal.get();
+
+        if (propertyGuard == null)
         {
-            return new HashSet<String>();
+            propertyGuard = new HashSet<String>();
+            propertyGuardThreadLocal.set(propertyGuard);
         }
-    };
+
+        return propertyGuard;
+    }
     
     private VariableResolver variableResolver;
 
@@ -87,12 +105,14 @@ public final class VariableResolverToELR
 
         final String strProperty = (String) property;
 
+        Collection<String> propertyGuard = getPropertyGuard();
+
         Object result = null;
         try
         {
             // only call the resolver if we haven't done it in current stack
-            if(!propertyGuard.get().contains(strProperty)) {
-                propertyGuard.get().add(strProperty);
+            if(!propertyGuard.contains(strProperty)) {
+                propertyGuard.add(strProperty);
                 result = variableResolver.resolveVariable(facesContext(context), strProperty);
             }
         }
@@ -113,7 +133,15 @@ public final class VariableResolverToELR
         }
         finally
         {
-            propertyGuard.get().remove(strProperty);
+            propertyGuard.remove(strProperty);
+
+            // if the propertyGuard is empty, remove the ThreadLocal
+            // in order to prevent a memory leak
+            if (propertyGuard.isEmpty())
+            {
+                propertyGuardThreadLocal.remove();
+            }
+
             // set property resolved to false in any case if result is null
             context.setPropertyResolved(result != null);
         }

Modified: myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/view/facelets/tag/MetaRulesetImpl.java
URL: http://svn.apache.org/viewvc/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/view/facelets/tag/MetaRulesetImpl.java?rev=1023364&r1=1023363&r2=1023364&view=diff
==============================================================================
--- myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/view/facelets/tag/MetaRulesetImpl.java (original)
+++ myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/view/facelets/tag/MetaRulesetImpl.java Sat Oct 16 19:40:14 2010
@@ -18,14 +18,8 @@
  */
 package org.apache.myfaces.view.facelets.tag;
 
-import java.beans.IntrospectionException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.WeakHashMap;
-import java.util.logging.Level;
-import java.util.logging.Logger;
+import org.apache.myfaces.shared_impl.util.ClassUtils;
+import org.apache.myfaces.view.facelets.util.ParameterCheck;
 
 import javax.faces.view.facelets.FaceletContext;
 import javax.faces.view.facelets.MetaRule;
@@ -35,8 +29,14 @@ import javax.faces.view.facelets.Metadat
 import javax.faces.view.facelets.Tag;
 import javax.faces.view.facelets.TagAttribute;
 import javax.faces.view.facelets.TagException;
-
-import org.apache.myfaces.view.facelets.util.ParameterCheck;
+import java.beans.IntrospectionException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * 
@@ -50,7 +50,8 @@ public final class MetaRulesetImpl exten
     //private final static Logger log = Logger.getLogger("facelets.tag.meta");
     private final static Logger log = Logger.getLogger(MetaRulesetImpl.class.getName());
 
-    private final static WeakHashMap<Class<?>, MetadataTarget> _metadata = new WeakHashMap<Class<?>, MetadataTarget>();
+    private final static WeakHashMap<ClassLoader, WeakHashMap<String, MetadataTarget>> _metadata
+            = new WeakHashMap<ClassLoader, WeakHashMap<String, MetadataTarget>>();
 
     private final Map<String, TagAttribute> _attributes;
 
@@ -181,7 +182,23 @@ public final class MetaRulesetImpl exten
 
     private final MetadataTarget _getMetadataTarget()
     {
-        MetadataTarget meta = _metadata.get(_type);
+        ClassLoader classLoader = ClassUtils.getContextClassLoader();
+        String metaKey = _type.getName();
+        MetadataTarget meta = null;
+
+        WeakHashMap<String, MetadataTarget> classLoaderMetadata = _metadata.get(classLoader);
+        if (classLoaderMetadata == null)
+        {
+            // first time with this ClassLoader - create new (inner) WeakHashMap
+            classLoaderMetadata = new WeakHashMap<String, MetadataTarget>();
+            _metadata.put(classLoader, classLoaderMetadata);
+        }
+        else
+        {
+            // we already have a inner WeakHashMap, try to get the existing metadata
+            meta = classLoaderMetadata.get(metaKey);
+        }
+
         if (meta == null)
         {
             try
@@ -193,7 +210,7 @@ public final class MetaRulesetImpl exten
                 throw new TagException(_tag, "Error Creating TargetMetadata", e);
             }
 
-            _metadata.put(_type, meta);
+            classLoaderMetadata.put(metaKey, meta);
         }
 
         return meta;