Author: niallp
Date: Mon Apr 21 11:05:18 2008
New Revision: 650215
URL: http://svn.apache.org/viewvc?rev=650215&view=rev
Log:
BEANUTILS-291 - fix Memory leaks - thanks to Clebert Suconic reporting, patches
and review:
ConvertUtilsBean - use WeakHashMap for converters cache
LocaleConvertUtilsBean - use new WeakFastHashMap for converters cache
MappedPropertyDescriptor - store method references in SoftReference and provide
mechanism to re-create
MethodUtils - put MethodDescriptors in WeakReference (within the WeakHashMap)
PropertyUtilsBean - use WeakHashMap for the two property descriptors caches
WrapDynaClass - store the class in a SoftReference
Modified:
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/ConvertUtilsBean.java
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/MappedPropertyDescriptor.java
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/MethodUtils.java
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/PropertyUtilsBean.java
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/WrapDynaClass.java
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/locale/LocaleConvertUtilsBean.java
commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/memoryleaktests/MemoryLeakTestCase.java
Modified:
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/ConvertUtilsBean.java
URL:
http://svn.apache.org/viewvc/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/ConvertUtilsBean.java?rev=650215&r1=650214&r2=650215&view=diff
==============================================================================
---
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/ConvertUtilsBean.java
(original)
+++
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/ConvertUtilsBean.java
Mon Apr 21 11:05:18 2008
@@ -28,7 +28,8 @@
import java.util.Calendar;
import java.util.Collection;
import java.util.Map;
-import java.util.HashMap;
+import java.util.WeakHashMap;
+
import org.apache.commons.beanutils.converters.ArrayConverter;
import org.apache.commons.beanutils.converters.BigDecimalConverter;
import org.apache.commons.beanutils.converters.BigIntegerConverter;
@@ -150,7 +151,7 @@
* The set of [EMAIL PROTECTED] Converter}s that can be used to convert
Strings
* into objects of a specified Class, keyed by the destination Class.
*/
- private Map converters = new HashMap();
+ private Map converters = new WeakHashMap();
/**
* The <code>Log</code> instance for this class.
Modified:
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/MappedPropertyDescriptor.java
URL:
http://svn.apache.org/viewvc/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/MappedPropertyDescriptor.java?rev=650215&r1=650214&r2=650215&view=diff
==============================================================================
---
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/MappedPropertyDescriptor.java
(original)
+++
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/MappedPropertyDescriptor.java
Mon Apr 21 11:05:18 2008
@@ -20,6 +20,9 @@
import java.beans.IntrospectionException;
import java.beans.PropertyDescriptor;
+import java.lang.ref.Reference;
+import java.lang.ref.SoftReference;
+import java.lang.ref.WeakReference;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
@@ -49,17 +52,17 @@
/**
* The underlying data type of the property we are describing.
*/
- private Class mappedPropertyType;
+ private Reference mappedPropertyTypeRef;
/**
* The reader method for this property (if any).
*/
- private Method mappedReadMethod;
+ private MappedMethodReference mappedReadMethodRef;
/**
* The writer method for this property (if any).
*/
- private Method mappedWriteMethod;
+ private MappedMethodReference mappedWriteMethodRef;
/**
* The parameter types array for the reader method signature.
@@ -98,6 +101,8 @@
String base = capitalizePropertyName(propertyName);
// Look for mapped read method and matching write method
+ Method mappedReadMethod = null;
+ Method mappedWriteMethod = null;
try {
try {
mappedReadMethod = getMethod(beanClass, "get" + base,
@@ -124,6 +129,8 @@
"' not found on " +
beanClass.getName());
}
+ mappedReadMethodRef = new MappedMethodReference(mappedReadMethod);
+ mappedWriteMethodRef = new MappedMethodReference(mappedWriteMethod);
findMappedPropertyType();
}
@@ -159,6 +166,8 @@
setName(propertyName);
// search the mapped get and set methods
+ Method mappedReadMethod = null;
+ Method mappedWriteMethod = null;
mappedReadMethod =
getMethod(beanClass, mappedGetterName, STRING_CLASS_PARAMETER);
@@ -170,6 +179,8 @@
mappedWriteMethod =
getMethod(beanClass, mappedSetterName, 2);
}
+ mappedReadMethodRef = new MappedMethodReference(mappedReadMethod);
+ mappedWriteMethodRef = new MappedMethodReference(mappedWriteMethod);
findMappedPropertyType();
}
@@ -200,8 +211,8 @@
}
setName(propertyName);
- mappedReadMethod = mappedGetter;
- mappedWriteMethod = mappedSetter;
+ mappedReadMethodRef = new MappedMethodReference(mappedGetter);
+ mappedWriteMethodRef = new MappedMethodReference(mappedSetter);
findMappedPropertyType();
}
@@ -218,7 +229,7 @@
* This is the type that will be returned by the mappedReadMethod.
*/
public Class getMappedPropertyType() {
- return mappedPropertyType;
+ return (Class)mappedPropertyTypeRef.get();
}
/**
@@ -228,7 +239,7 @@
* May return null if the property can't be read.
*/
public Method getMappedReadMethod() {
- return mappedReadMethod;
+ return mappedReadMethodRef.get();
}
/**
@@ -240,7 +251,7 @@
*/
public void setMappedReadMethod(Method mappedGetter)
throws IntrospectionException {
- mappedReadMethod = mappedGetter;
+ mappedReadMethodRef = new MappedMethodReference(mappedGetter);
findMappedPropertyType();
}
@@ -251,7 +262,7 @@
* May return null if the property can't be written.
*/
public Method getMappedWriteMethod() {
- return mappedWriteMethod;
+ return mappedWriteMethodRef.get();
}
/**
@@ -263,7 +274,7 @@
*/
public void setMappedWriteMethod(Method mappedSetter)
throws IntrospectionException {
- mappedWriteMethod = mappedSetter;
+ mappedWriteMethodRef = new MappedMethodReference(mappedSetter);
findMappedPropertyType();
}
@@ -275,7 +286,9 @@
*/
private void findMappedPropertyType() throws IntrospectionException {
try {
- mappedPropertyType = null;
+ Method mappedReadMethod = getMappedReadMethod();
+ Method mappedWriteMethod = getMappedWriteMethod();
+ Class mappedPropertyType = null;
if (mappedReadMethod != null) {
if (mappedReadMethod.getParameterTypes().length != 1) {
throw new IntrospectionException
@@ -302,6 +315,7 @@
}
mappedPropertyType = params[1];
}
+ mappedPropertyTypeRef = new SoftReference(mappedPropertyType);
} catch (IntrospectionException ex) {
throw ex;
}
@@ -404,4 +418,60 @@
"\" with " + parameterCount + " parameter(s) of matching
types.");
}
+ /**
+ * Holds a [EMAIL PROTECTED] Method} in a [EMAIL PROTECTED] SoftReference}
so that it
+ * it doesn't prevent any ClassLoader being garbage collected, but
+ * tries to re-create the method if the method reference has been
+ * released.
+ *
+ * See http://issues.apache.org/jira/browse/BEANUTILS-291
+ */
+ private static class MappedMethodReference {
+ private String className;
+ private String methodName;
+ private Reference methodRef;
+ private Reference classRef;
+ private Reference writeParamTypeRef;
+ MappedMethodReference(Method m) {
+ if (m != null) {
+ className = m.getDeclaringClass().getName();
+ methodName = m.getName();
+ methodRef = new SoftReference(m);
+ classRef = new WeakReference(m.getDeclaringClass());
+ Class[] types = m.getParameterTypes();
+ if (types.length == 2) {
+ writeParamTypeRef = new WeakReference(types[1]);
+ }
+ }
+ }
+ private Method get() {
+ if (methodRef == null) {
+ return null;
+ }
+ Method m = (Method)methodRef.get();
+ if (m == null) {
+ Class clazz = (Class)classRef.get();
+ if (clazz == null) {
+ throw new RuntimeException("Method " + methodName + " for
" +
+ className + " could not be reconstructed - class
reference has gone");
+ }
+ Class[] paramTypes = null;
+ if (writeParamTypeRef != null) {
+ paramTypes = new Class[] {String.class,
(Class)writeParamTypeRef.get()};
+ } else {
+ paramTypes = STRING_CLASS_PARAMETER;
+ }
+ try {
+ m = clazz.getMethod(methodName, paramTypes);
+ // Un-comment following line for testing
+ // System.out.println("Recreated Method " + methodName + "
for " + className);
+ } catch (NoSuchMethodException e) {
+ throw new RuntimeException("Method " + methodName + " for
" +
+ className + " could not be reconstructed - method
not found");
+ }
+ methodRef = new SoftReference(m);
+ }
+ return m;
+ }
+ }
}
Modified:
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/MethodUtils.java
URL:
http://svn.apache.org/viewvc/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/MethodUtils.java?rev=650215&r1=650214&r2=650215&view=diff
==============================================================================
---
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/MethodUtils.java
(original)
+++
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/MethodUtils.java
Mon Apr 21 11:05:18 2008
@@ -18,6 +18,8 @@
package org.apache.commons.beanutils;
+import java.lang.ref.Reference;
+import java.lang.ref.WeakReference;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
@@ -1262,7 +1264,10 @@
*/
private static Method getCachedMethod(MethodDescriptor md) {
if (CACHE_METHODS) {
- return (Method)cache.get(md);
+ Reference methodRef = (Reference)cache.get(md);
+ if (methodRef != null) {
+ return (Method)methodRef.get();
+ }
}
return null;
}
@@ -1276,7 +1281,7 @@
private static void cacheMethod(MethodDescriptor md, Method method) {
if (CACHE_METHODS) {
if (method != null) {
- cache.put(md, method);
+ cache.put(md, new WeakReference(method));
}
}
}
Modified:
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/PropertyUtilsBean.java
URL:
http://svn.apache.org/viewvc/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/PropertyUtilsBean.java?rev=650215&r1=650214&r2=650215&view=diff
==============================================================================
---
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/PropertyUtilsBean.java
(original)
+++
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/PropertyUtilsBean.java
Mon Apr 21 11:05:18 2008
@@ -30,6 +30,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.WeakHashMap;
import org.apache.commons.beanutils.expression.DefaultResolver;
import org.apache.commons.beanutils.expression.Resolver;
@@ -120,8 +121,8 @@
* The cache of PropertyDescriptor arrays for beans we have already
* introspected, keyed by the java.lang.Class of this object.
*/
- private FastHashMap descriptorsCache = null;
- private FastHashMap mappedDescriptorsCache = null;
+ private Map descriptorsCache = new WeakHashMap();
+ private Map mappedDescriptorsCache = new WeakHashMap();
private static final Class[] EMPTY_CLASS_PARAMETERS = new Class[0];
private static final Class[] LIST_CLASS_PARAMETER = new Class[]
{java.util.List.class};
@@ -135,10 +136,6 @@
/** Base constructor */
public PropertyUtilsBean() {
- descriptorsCache = new FastHashMap();
- descriptorsCache.setFast(true);
- mappedDescriptorsCache = new FastHashMap();
- mappedDescriptorsCache.setFast(true);
}
Modified:
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/WrapDynaClass.java
URL:
http://svn.apache.org/viewvc/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/WrapDynaClass.java?rev=650215&r1=650214&r2=650215&view=diff
==============================================================================
---
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/WrapDynaClass.java
(original)
+++
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/WrapDynaClass.java
Mon Apr 21 11:05:18 2008
@@ -19,6 +19,8 @@
import java.beans.PropertyDescriptor;
+import java.lang.ref.Reference;
+import java.lang.ref.SoftReference;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
@@ -61,7 +63,8 @@
*/
private WrapDynaClass(Class beanClass) {
- this.beanClass = beanClass;
+ this.beanClassRef = new SoftReference(beanClass);
+ this.beanClassName = beanClass.getName();
introspect();
}
@@ -69,10 +72,21 @@
// ----------------------------------------------------- Instance Variables
+ /**
+ * Name of the JavaBean class represented by this WrapDynaClass.
+ */
+ private String beanClassName = null;
+
+ /**
+ * Reference to the JavaBean class represented by this WrapDynaClass.
+ */
+ private Reference beanClassRef = null;
/**
* The JavaBean <code>Class</code> which is represented by this
* <code>WrapDynaClass</code>.
+ *
+ * @deprecated No longer initialized, use getBeanClass() method instead
*/
protected Class beanClass = null;
@@ -206,6 +220,14 @@
// ------------------------------------------------------ DynaClass Methods
+ /**
+ * Return the class of the underlying wrapped bean.
+ *
+ * @return the class of the underlying wrapped bean
+ */
+ protected Class getBeanClass() {
+ return (Class)beanClassRef.get();
+ }
/**
* Return the name of this DynaClass (analogous to the
@@ -217,7 +239,7 @@
*/
public String getName() {
- return (this.beanClass.getName());
+ return beanClassName;
}
@@ -289,7 +311,7 @@
public DynaBean newInstance()
throws IllegalAccessException, InstantiationException {
- return new WrapDynaBean(beanClass.newInstance());
+ return new WrapDynaBean(getBeanClass().newInstance());
}
@@ -353,6 +375,7 @@
protected void introspect() {
// Look up the property descriptors for this bean class
+ Class beanClass = getBeanClass();
PropertyDescriptor[] regulars =
PropertyUtils.getPropertyDescriptors(beanClass);
if (regulars == null) {
Modified:
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/locale/LocaleConvertUtilsBean.java
URL:
http://svn.apache.org/viewvc/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/locale/LocaleConvertUtilsBean.java?rev=650215&r1=650214&r2=650215&view=diff
==============================================================================
---
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/locale/LocaleConvertUtilsBean.java
(original)
+++
commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/locale/LocaleConvertUtilsBean.java
Mon Apr 21 11:05:18 2008
@@ -37,7 +37,12 @@
import java.lang.reflect.Array;
import java.math.BigDecimal;
import java.math.BigInteger;
+import java.util.Collection;
+import java.util.Collections;
import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.WeakHashMap;
/**
* <p>Utility methods for converting locale-sensitive String scalar values to
objects of the
@@ -454,7 +459,7 @@
*/
protected FastHashMap create(Locale locale) {
- FastHashMap converter = new FastHashMap();
+ FastHashMap converter = new WeakFastHashMap();
converter.setFast(false);
converter.put(BigDecimal.class, new BigDecimalLocaleConverter(locale,
applyLocalized));
@@ -491,5 +496,68 @@
converter.setFast(true);
return converter;
+ }
+
+ /**
+ * FastHashMap implementation that uses WeakReferences to overcome
+ * memory leak problems.
+ *
+ * This is a hack to retain binary compatibility with previous
+ * releases (where FastHashMap is exposed in the API), but
+ * use WeakHashMap to resolve memory leaks.
+ */
+ private static class WeakFastHashMap extends FastHashMap {
+
+ private Map fastMap = new WeakHashMap();
+ private Map slowMap = Collections.synchronizedMap(fastMap);
+
+ private WeakFastHashMap() {
+ super(0);
+ }
+ public void clear() {
+ getMap().clear();
+ }
+ public boolean containsKey(Object key) {
+ return getMap().containsKey(key);
+ }
+ public boolean containsValue(Object value) {
+ return getMap().containsValue(value);
+ }
+ public Set entrySet() {
+ return getMap().entrySet();
+ }
+ public boolean equals(Object o) {
+ return getMap().equals(o);
+ }
+ public Object get(Object key) {
+ return getMap().get(key);
+ }
+ public int hashCode() {
+ return getMap().hashCode();
+ }
+ public boolean isEmpty() {
+ return getMap().isEmpty();
+ }
+ public Set keySet() {
+ return getMap().keySet();
+ }
+ public Object put(Object key, Object value) {
+ return getMap().put(key, value);
+ }
+ public void putAll(Map m) {
+ getMap().putAll(m);
+ }
+ public Object remove(Object key) {
+ return getMap().remove(key);
+ }
+ public int size() {
+ return getMap().size();
+ }
+ public Collection values() {
+ return getMap().values();
+ }
+ private Map getMap() {
+ return (getFast() ? fastMap : slowMap);
+ }
}
}
Modified:
commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/memoryleaktests/MemoryLeakTestCase.java
URL:
http://svn.apache.org/viewvc/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/memoryleaktests/MemoryLeakTestCase.java?rev=650215&r1=650214&r2=650215&view=diff
==============================================================================
---
commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/memoryleaktests/MemoryLeakTestCase.java
(original)
+++
commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/memoryleaktests/MemoryLeakTestCase.java
Mon Apr 21 11:05:18 2008
@@ -21,11 +21,13 @@
import java.net.URL;
import java.net.URLClassLoader;
import java.util.Locale;
+import java.util.StringTokenizer;
import junit.framework.TestCase;
import org.apache.commons.beanutils.BeanUtilsBean;
import org.apache.commons.beanutils.ConvertUtils;
+import org.apache.commons.beanutils.MappedPropertyDescriptor;
import org.apache.commons.beanutils.MethodUtils;
import org.apache.commons.beanutils.PropertyUtils;
import org.apache.commons.beanutils.WrapDynaBean;
@@ -48,6 +50,9 @@
* Tests that PropertyUtilsBean's descriptorsCache doesn't cause a memory
leak.
*/
public void testPropertyUtilsBean_descriptorsCache_memoryLeak() throws
Exception {
+ if (isPre15JVM()) {
+ return;
+ }
// Clear All BeanUtils caches before the test
clearAllBeanUtilsCaches();
@@ -94,6 +99,9 @@
* Tests that PropertyUtilsBean's mappedDescriptorsCache doesn't cause a
memory leak.
*/
public void testPropertyUtilsBean_mappedDescriptorsCache_memoryLeak()
throws Exception {
+ if (isPre15JVM()) {
+ return;
+ }
// Clear All BeanUtils caches before the test
clearAllBeanUtilsCaches();
@@ -143,6 +151,50 @@
}
/**
+ * Tests that MappedPropertyDescriptor can re-create the Method reference
after it
+ * has been garbage collected.
+ */
+ public void testMappedPropertyDescriptor_MappedMethodReference() throws
Exception {
+
+ // Clear All BeanUtils caches before the test
+ clearAllBeanUtilsCaches();
+
+ String className =
"org.apache.commons.beanutils.memoryleaktests.pojotests.SomeMappedPojo";
+ ClassLoader loader = newClassLoader();
+ Class beanClass = loader.loadClass(className);
+ Object bean = beanClass.newInstance();
+ //
-----------------------------------------------------------------------------
+
+ // Sanity checks only
+ assertNotNull("ClassLoader is null", loader);
+ assertNotNull("BeanClass is null", beanClass);
+ assertNotSame("ClassLoaders should be different..",
getClass().getClassLoader(), beanClass.getClassLoader());
+ assertSame("BeanClass ClassLoader incorrect",
beanClass.getClassLoader(), loader);
+
+ MappedPropertyDescriptor descriptor = new
MappedPropertyDescriptor("mappedProperty", beanClass);
+ assertNotNull("1-Read Method null", descriptor.getMappedReadMethod());
+ assertNotNull("1-Write Method null",
descriptor.getMappedWriteMethod());
+ assertEquals("1-Read Method name", "getMappedProperty",
descriptor.getMappedReadMethod().getName());
+ assertEquals("1-Read Write name", "setMappedProperty",
descriptor.getMappedWriteMethod().getName());
+
+ forceGarbageCollection(); /* Try to force the garbage collector to run
by filling up memory */
+
+ // The aim of this test is to check the functinality in
MappedPropertyDescriptor which
+ // re-creates the Method references after they have been garbage
collected. However theres no
+ // way of knowing the method references were garbage collected and
that code was run, except by
+ // un-commeting the System.out statement in MappedPropertyDescriptor's
MappedMethodReference's
+ // get() method.
+
+ assertNotNull("1-Read Method null", descriptor.getMappedReadMethod());
+ assertNotNull("1-Write Method null",
descriptor.getMappedWriteMethod());
+ assertEquals("1-Read Method name", "getMappedProperty",
descriptor.getMappedReadMethod().getName());
+ assertEquals("1-Read Write name", "setMappedProperty",
descriptor.getMappedWriteMethod().getName());
+
+ // Clear All BeanUtils caches after the test
+ clearAllBeanUtilsCaches();
+ }
+
+ /**
* Tests that MethodUtils's cache doesn't cause a memory leak.
*/
public void testMethodUtils_cache_memoryLeak() throws Exception {
@@ -192,6 +244,9 @@
* Tests that WrapDynaClass's dynaClasses doesn't cause a memory leak.
*/
public void testWrapDynaClass_dynaClasses_memoryLeak() throws Exception {
+ if (isPre15JVM()) {
+ return;
+ }
// Clear All BeanUtils caches before the test
clearAllBeanUtilsCaches();
@@ -426,5 +481,22 @@
System.out.println(jvmti.exploreClassReferences(className, 8, true,
true, true, false, false));
System.out.println(" ----------------" + test + " END
------------------");
*/
+ }
+
+ /**
+ * Test for JDK 1.5
+ */
+ private boolean isPre15JVM() {
+ String version = System.getProperty("java.specification.version");
+ StringTokenizer tokenizer = new StringTokenizer(version,".");
+ if (tokenizer.nextToken().equals("1")) {
+ String minorVersion = tokenizer.nextToken();
+ if (minorVersion.equals("0")) return true;
+ if (minorVersion.equals("1")) return true;
+ if (minorVersion.equals("2")) return true;
+ if (minorVersion.equals("3")) return true;
+ if (minorVersion.equals("4")) return true;
+ }
+ return false;
}
}
