You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aries.apache.org by ma...@apache.org on 2011/02/04 18:35:49 UTC
svn commit: r1067234 - in /aries/trunk/blueprint/blueprint-core/src:
main/java/org/apache/aries/blueprint/utils/ReflectionUtils.java
test/java/org/apache/aries/blueprint/utils/ReflectionUtilsTest.java
Author: mahrwald
Date: Fri Feb 4 17:35:49 2011
New Revision: 1067234
URL: http://svn.apache.org/viewvc?rev=1067234&view=rev
Log:
ARIES-568: Fix memory leak in storing bean method and field descriptors
Modified:
aries/trunk/blueprint/blueprint-core/src/main/java/org/apache/aries/blueprint/utils/ReflectionUtils.java
aries/trunk/blueprint/blueprint-core/src/test/java/org/apache/aries/blueprint/utils/ReflectionUtilsTest.java
Modified: aries/trunk/blueprint/blueprint-core/src/main/java/org/apache/aries/blueprint/utils/ReflectionUtils.java
URL: http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-core/src/main/java/org/apache/aries/blueprint/utils/ReflectionUtils.java?rev=1067234&r1=1067233&r2=1067234&view=diff
==============================================================================
--- aries/trunk/blueprint/blueprint-core/src/main/java/org/apache/aries/blueprint/utils/ReflectionUtils.java (original)
+++ aries/trunk/blueprint/blueprint-core/src/main/java/org/apache/aries/blueprint/utils/ReflectionUtils.java Fri Feb 4 17:35:49 2011
@@ -18,6 +18,7 @@
*/
package org.apache.aries.blueprint.utils;
+import java.lang.ref.WeakReference;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
@@ -26,7 +27,6 @@ import java.lang.reflect.Modifier;
import java.lang.reflect.Type;
import java.security.AccessControlContext;
import java.security.AccessController;
-import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.ArrayList;
@@ -53,7 +53,6 @@ import org.osgi.service.blueprint.contai
*/
public class ReflectionUtils {
- // TODO: MLK: PropertyDescriptor holds a reference to Method which holds a reference to the Class itself
private static Map<Class<?>, PropertyDescriptor[][]> beanInfos = Collections.synchronizedMap(new WeakHashMap<Class<?>, PropertyDescriptor[][]>());
public static boolean hasDefaultConstructor(Class type) {
@@ -360,11 +359,15 @@ public class ReflectionUtils {
}
private static class FieldPropertyDescriptor extends PropertyDescriptor {
- private final Field field;
+ // instead of holding on to the java.lang.reflect.Field objects we retrieve it every time. The reason is that PropertyDescriptors are
+ // used as values in a WeakHashMap with the class corresponding to the field as the key
+ private final String fieldName;
+ private final WeakReference<Class<?>> declaringClass;
public FieldPropertyDescriptor(String name, Field field) {
super(name);
- this.field = field;
+ this.fieldName = field.getName();
+ this.declaringClass = new WeakReference(field.getDeclaringClass());
}
public boolean allowsGet() {
@@ -374,14 +377,19 @@ public class ReflectionUtils {
public boolean allowsSet() {
return true;
}
+
+ private Field getField(ExtendedBlueprintContainer container) throws ClassNotFoundException, NoSuchFieldException {
+ if (declaringClass.get() == null) throw new ClassNotFoundException("Declaring class was garbage collected");
+
+ return declaringClass.get().getDeclaredField(fieldName);
+ }
- protected Object internalGet(ExtendedBlueprintContainer container, final Object instance) throws IllegalArgumentException, IllegalAccessException {
+ protected Object internalGet(final ExtendedBlueprintContainer container, final Object instance) throws Exception {
if (useContainersPermission(container)) {
try {
return AccessController.doPrivileged(new PrivilegedExceptionAction<Object>() {
public Object run() throws Exception {
- field.setAccessible(true);
- return field.get(instance);
+ return doInternalGet(container, instance);
}
});
} catch (PrivilegedActionException pae) {
@@ -390,19 +398,27 @@ public class ReflectionUtils {
else throw (RuntimeException) e;
}
} else {
- field.setAccessible(true);
+ return doInternalGet(container, instance);
+ }
+ }
+
+ private Object doInternalGet(ExtendedBlueprintContainer container, Object instance) throws Exception {
+ Field field = getField(container);
+ boolean isAccessible = field.isAccessible();
+ field.setAccessible(true);
+ try {
return field.get(instance);
+ } finally {
+ field.setAccessible(isAccessible);
}
}
- protected void internalSet(ExtendedBlueprintContainer container, final Object instance, Object value) throws Exception {
- final Object convertedValue = convert(value, field.getGenericType());
+ protected void internalSet(final ExtendedBlueprintContainer container, final Object instance, final Object value) throws Exception {
if (useContainersPermission(container)) {
try {
AccessController.doPrivileged(new PrivilegedExceptionAction<Object>() {
public Object run() throws Exception {
- field.setAccessible(true);
- field.set(instance, convertedValue);
+ doInternalSet(container, instance, value);
return null;
}
});
@@ -410,8 +426,19 @@ public class ReflectionUtils {
throw pae.getException();
}
} else {
- field.setAccessible(true);
+ doInternalSet(container, instance, value);
+ }
+ }
+
+ private void doInternalSet(ExtendedBlueprintContainer container, Object instance, Object value) throws Exception {
+ Field field = getField(container);
+ final Object convertedValue = convert(value, field.getGenericType());
+ boolean isAccessible = field.isAccessible();
+ field.setAccessible(true);
+ try {
field.set(instance, convertedValue);
+ } finally {
+ field.setAccessible(isAccessible);
}
}
@@ -423,12 +450,9 @@ public class ReflectionUtils {
* @param container
* @return
*/
- private boolean useContainersPermission(ExtendedBlueprintContainer container) {
- ClassLoader loader = AccessController.doPrivileged(new PrivilegedAction<ClassLoader>() {
- public ClassLoader run() {
- return field.getDeclaringClass().getClassLoader();
- }
- });
+ private boolean useContainersPermission(ExtendedBlueprintContainer container) throws ClassNotFoundException {
+ if (declaringClass.get() == null) throw new ClassNotFoundException("Declaring class was garbage collected");
+ ClassLoader loader = declaringClass.get().getClassLoader();
if (loader == null) return false;
@@ -441,14 +465,67 @@ public class ReflectionUtils {
}
}
+ private static class MethodDescriptor {
+ private final String methodName;
+ private final WeakReference<Class<?>> declaringClass;
+ private final List<WeakReference<Class<?>>> argClasses;
+
+ public MethodDescriptor(Method method) {
+ methodName = method.getName();
+ declaringClass = new WeakReference<Class<?>>(method.getDeclaringClass());
+
+ List<WeakReference<Class<?>>> accumulator = new ArrayList<WeakReference<Class<?>>>();
+ for (Class<?> c : method.getParameterTypes()) {
+ accumulator.add(new WeakReference<Class<?>>(c));
+ }
+ argClasses = Collections.unmodifiableList(accumulator);
+ }
+
+ public Method getMethod(ExtendedBlueprintContainer container) throws ClassNotFoundException, NoSuchMethodException {
+ Class<?>[] argumentClasses = new Class<?>[argClasses.size()];
+ for (int i=0; i<argClasses.size(); i++) {
+ argumentClasses[i] = argClasses.get(i).get();
+ if (argumentClasses[i] == null) throw new ClassNotFoundException("Argument class was garbage collected");
+ }
+
+ if (declaringClass.get() == null) throw new ClassNotFoundException("Declaring class was garbage collected");
+
+ return declaringClass.get().getMethod(methodName, argumentClasses);
+ }
+
+ public String toString() {
+ StringBuilder builder = new StringBuilder();
+ builder.append(declaringClass.get()).append(".").append(methodName).append("(");
+
+ boolean first = true;
+ for (WeakReference<Class<?>> wcl : argClasses) {
+ if (!!!first) builder.append(",");
+ else first = false;
+
+ builder.append(wcl.get());
+ }
+
+ return builder.toString();
+ }
+ }
+
private static class MethodPropertyDescriptor extends PropertyDescriptor {
- private final Method getter;
- private final Collection<Method> setters;
+ // instead of holding on to the java.lang.reflect.Method objects we retrieve it every time. The reason is that PropertyDescriptors are
+ // used as values in a WeakHashMap with the class corresponding to the methods as the key
+ private final MethodDescriptor getter;
+ private final Collection<MethodDescriptor> setters;
private MethodPropertyDescriptor(String name, Method getter, Collection<Method> setters) {
super(name);
- this.getter = getter;
- this.setters = (setters != null) ? setters : Collections.<Method>emptyList();
+ this.getter = (getter != null) ? new MethodDescriptor(getter) : null;
+
+ if (setters != null) {
+ Collection<MethodDescriptor> accumulator = new ArrayList<MethodDescriptor>();
+ for (Method s : setters) accumulator.add(new MethodDescriptor(s));
+ this.setters = Collections.unmodifiableCollection(accumulator);
+ } else {
+ this.setters = Collections.emptyList();
+ }
}
public boolean allowsGet() {
@@ -460,9 +537,9 @@ public class ReflectionUtils {
}
protected Object internalGet(ExtendedBlueprintContainer container, Object instance)
- throws IllegalArgumentException, IllegalAccessException, InvocationTargetException {
+ throws Exception {
if (getter != null) {
- return getter.invoke(instance);
+ return getter.getMethod(container).invoke(instance);
} else {
throw new UnsupportedOperationException();
}
@@ -470,7 +547,7 @@ public class ReflectionUtils {
protected void internalSet(ExtendedBlueprintContainer container, Object instance, Object value) throws Exception {
- Method setterMethod = findSetter(value);
+ Method setterMethod = findSetter(container, value);
if (setterMethod != null) {
setterMethod.invoke(instance, convert(value, setterMethod.getGenericParameterTypes()[0]));
@@ -481,31 +558,41 @@ public class ReflectionUtils {
}
}
- private Method findSetter(Object value) {
+ private Method findSetter(ExtendedBlueprintContainer container, Object value) throws Exception {
Class<?> valueType = (value == null) ? null : value.getClass();
- Method result = findMethodByClass(valueType);
+ Method getterMethod = (getter != null) ? getter.getMethod(container) : null;
+ Collection<Method> setterMethods = getSetters(container);
+
+ Method result = findMethodByClass(getterMethod, setterMethods, valueType);
- if (result == null) result = findMethodWithConversion(value);
+ if (result == null) result = findMethodWithConversion(setterMethods, value);
return result;
}
- private Method findMethodByClass(Class<?> arg)
+ private Collection<Method> getSetters(ExtendedBlueprintContainer container) throws Exception {
+ Collection<Method> result = new ArrayList<Method>();
+ for (MethodDescriptor md : setters) result.add(md.getMethod(container));
+
+ return result;
+ }
+
+ private Method findMethodByClass(Method getterMethod, Collection<Method> setterMethods, Class<?> arg)
throws ComponentDefinitionException {
Method result = null;
- if (!hasSameTypeSetter()) {
+ if (!hasSameTypeSetter(getterMethod, setterMethods)) {
throw new ComponentDefinitionException(
"At least one Setter method has to match the type of the Getter method for property "
+ getName());
}
- if (setters.size() == 1) {
- return setters.iterator().next();
+ if (setterMethods.size() == 1) {
+ return setterMethods.iterator().next();
}
- for (Method m : setters) {
+ for (Method m : setterMethods) {
Class<?> paramType = m.getParameterTypes()[0];
if ((arg == null && Object.class.isAssignableFrom(paramType))
@@ -536,24 +623,25 @@ public class ReflectionUtils {
}
// ensure there is a setter that matches the type of the getter
- private boolean hasSameTypeSetter() {
- if (getter == null) {
+ private boolean hasSameTypeSetter(Method getterMethod, Collection<Method> setterMethods) {
+ if (getterMethod == null) {
return true;
}
- Iterator<Method> it = setters.iterator();
+
+ Iterator<Method> it = setterMethods.iterator();
while (it.hasNext()) {
Method m = it.next();
- if (m.getParameterTypes()[0].equals(getter.getReturnType())) {
+ if (m.getParameterTypes()[0].equals(getterMethod.getReturnType())) {
return true;
}
}
return false;
}
- private Method findMethodWithConversion(Object value) throws ComponentDefinitionException {
+ private Method findMethodWithConversion(Collection<Method> setterMethods, Object value) throws Exception {
ExecutionContext ctx = ExecutionContext.Holder.getContext();
List<Method> matchingMethods = new ArrayList<Method>();
- for (Method m : setters) {
+ for (Method m : setterMethods) {
Type paramType = m.getGenericParameterTypes()[0];
if (ctx.canConvert(value, new GenericType(paramType))) matchingMethods.add(m);
}
Modified: aries/trunk/blueprint/blueprint-core/src/test/java/org/apache/aries/blueprint/utils/ReflectionUtilsTest.java
URL: http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-core/src/test/java/org/apache/aries/blueprint/utils/ReflectionUtilsTest.java?rev=1067234&r1=1067233&r2=1067234&view=diff
==============================================================================
--- aries/trunk/blueprint/blueprint-core/src/test/java/org/apache/aries/blueprint/utils/ReflectionUtilsTest.java (original)
+++ aries/trunk/blueprint/blueprint-core/src/test/java/org/apache/aries/blueprint/utils/ReflectionUtilsTest.java Fri Feb 4 17:35:49 2011
@@ -43,7 +43,13 @@ import static org.junit.Assert.*;
public class ReflectionUtilsTest {
private PropertyDescriptor[] sut;
- private final ExtendedBlueprintContainer mockBlueprint = Skeleton.newMock(ExtendedBlueprintContainer.class);
+ private final ExtendedBlueprintContainer mockBlueprint = Skeleton.newMock(
+ new Object() {
+ public Class<?> loadClass(String name) throws ClassNotFoundException {
+ return Thread.currentThread().getContextClassLoader().loadClass(name);
+ }
+ },
+ ExtendedBlueprintContainer.class);
static class GetterOnly {
public String getValue() { return "test"; }