From 69190da5ef104227fad08bfca46bbd10e575698b Mon Sep 17 00:00:00 2001 From: Sergey Malenkov Date: Fri, 27 Nov 2009 15:24:43 +0300 Subject: [PATCH] 5102804: Memory leak in Introspector.getBeanInfo(Class) for custom BeanInfo: Class param Reviewed-by: peterz --- .../classes/com/sun/beans/WeakCache.java | 7 + .../classes/java/beans/Introspector.java | 95 +++++------ .../java/beans/Introspector/Test5102804.java | 155 ++++++++++++++++++ 3 files changed, 201 insertions(+), 56 deletions(-) create mode 100644 jdk/test/java/beans/Introspector/Test5102804.java diff --git a/jdk/src/share/classes/com/sun/beans/WeakCache.java b/jdk/src/share/classes/com/sun/beans/WeakCache.java index 461c48e1fd3..3aa6373dc46 100644 --- a/jdk/src/share/classes/com/sun/beans/WeakCache.java +++ b/jdk/src/share/classes/com/sun/beans/WeakCache.java @@ -81,4 +81,11 @@ public final class WeakCache { this.map.remove(key); } } + + /** + * Removes all of the mappings from this cache. + */ + public void clear() { + this.map.clear(); + } } diff --git a/jdk/src/share/classes/java/beans/Introspector.java b/jdk/src/share/classes/java/beans/Introspector.java index 3d56599576d..f0ce2a85b81 100644 --- a/jdk/src/share/classes/java/beans/Introspector.java +++ b/jdk/src/share/classes/java/beans/Introspector.java @@ -25,26 +25,19 @@ package java.beans; +import com.sun.beans.WeakCache; import com.sun.beans.finder.BeanInfoFinder; import com.sun.beans.finder.ClassFinder; -import java.lang.ref.Reference; -import java.lang.ref.SoftReference; - import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.security.AccessController; -import java.security.PrivilegedAction; - -import java.util.Collections; import java.util.Map; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; import java.util.EventListener; import java.util.List; -import java.util.WeakHashMap; import java.util.TreeMap; import sun.awt.AppContext; @@ -112,8 +105,8 @@ public class Introspector { public final static int IGNORE_ALL_BEANINFO = 3; // Static Caches to speed up introspection. - private static Map declaredMethodCache = - Collections.synchronizedMap(new WeakHashMap()); + private static WeakCache, Method[]> declaredMethodCache = + new WeakCache, Method[]>(); private static final Object BEANINFO_CACHE = new Object(); @@ -174,20 +167,21 @@ public class Introspector { if (!ReflectUtil.isPackageAccessible(beanClass)) { return (new Introspector(beanClass, null, USE_ALL_BEANINFO)).getBeanInfo(); } - Map, BeanInfo> map; synchronized (BEANINFO_CACHE) { - map = (Map, BeanInfo>) AppContext.getAppContext().get(BEANINFO_CACHE); - if (map == null) { - map = Collections.synchronizedMap(new WeakHashMap, BeanInfo>()); - AppContext.getAppContext().put(BEANINFO_CACHE, map); + WeakCache, BeanInfo> beanInfoCache = + (WeakCache, BeanInfo>) AppContext.getAppContext().get(BEANINFO_CACHE); + + if (beanInfoCache == null) { + beanInfoCache = new WeakCache, BeanInfo>(); + AppContext.getAppContext().put(BEANINFO_CACHE, beanInfoCache); } + BeanInfo beanInfo = beanInfoCache.get(beanClass); + if (beanInfo == null) { + beanInfo = (new Introspector(beanClass, null, USE_ALL_BEANINFO)).getBeanInfo(); + beanInfoCache.put(beanClass, beanInfo); + } + return beanInfo; } - BeanInfo bi = map.get(beanClass); - if (bi == null) { - bi = (new Introspector(beanClass, null, USE_ALL_BEANINFO)).getBeanInfo(); - map.put(beanClass, bi); - } - return bi; } /** @@ -359,11 +353,13 @@ public class Introspector { */ public static void flushCaches() { - Map map = (Map) AppContext.getAppContext().get(BEANINFO_CACHE); - if (map != null) { - map.clear(); + synchronized (BEANINFO_CACHE) { + WeakCache beanInfoCache = (WeakCache) AppContext.getAppContext().get(BEANINFO_CACHE); + if (beanInfoCache != null) { + beanInfoCache.clear(); + } + declaredMethodCache.clear(); } - declaredMethodCache.clear(); } /** @@ -385,11 +381,13 @@ public class Introspector { if (clz == null) { throw new NullPointerException(); } - Map map = (Map) AppContext.getAppContext().get(BEANINFO_CACHE); - if (map != null) { - map.remove(clz); + synchronized (BEANINFO_CACHE) { + WeakCache beanInfoCache = (WeakCache) AppContext.getAppContext().get(BEANINFO_CACHE); + if (beanInfoCache != null) { + beanInfoCache.put(clz, null); + } + declaredMethodCache.put(clz, null); } - declaredMethodCache.remove(clz); } //====================================================================== @@ -1272,41 +1270,26 @@ public class Introspector { /* * Internal method to return *public* methods within a class. */ - private static synchronized Method[] getPublicDeclaredMethods(Class clz) { + private static Method[] getPublicDeclaredMethods(Class clz) { // Looking up Class.getDeclaredMethods is relatively expensive, // so we cache the results. - Method[] result = null; if (!ReflectUtil.isPackageAccessible(clz)) { return new Method[0]; } - final Class fclz = clz; - Reference ref = (Reference)declaredMethodCache.get(fclz); - if (ref != null) { - result = (Method[])ref.get(); - if (result != null) { - return result; - } - } - - // We have to raise privilege for getDeclaredMethods - result = (Method[]) AccessController.doPrivileged(new PrivilegedAction() { - public Object run() { - return fclz.getDeclaredMethods(); + synchronized (BEANINFO_CACHE) { + Method[] result = declaredMethodCache.get(clz); + if (result == null) { + result = clz.getMethods(); + for (int i = 0; i < result.length; i++) { + Method method = result[i]; + if (!method.getDeclaringClass().equals(clz)) { + result[i] = null; + } } - }); - - - // Null out any non-public methods. - for (int i = 0; i < result.length; i++) { - Method method = result[i]; - int mods = method.getModifiers(); - if (!Modifier.isPublic(mods)) { - result[i] = null; + declaredMethodCache.put(clz, result); } + return result; } - // Add it to the cache. - declaredMethodCache.put(fclz, new SoftReference(result)); - return result; } //====================================================================== diff --git a/jdk/test/java/beans/Introspector/Test5102804.java b/jdk/test/java/beans/Introspector/Test5102804.java new file mode 100644 index 00000000000..71f23f92941 --- /dev/null +++ b/jdk/test/java/beans/Introspector/Test5102804.java @@ -0,0 +1,155 @@ +/* + * Copyright 2009 Sun Microsystems, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +/* + * @test + * @bug 5102804 + * @summary Tests memory leak + * @author Sergey Malenkov + */ + +import java.beans.BeanInfo; +import java.beans.IntrospectionException; +import java.beans.Introspector; +import java.beans.PropertyDescriptor; +import java.beans.SimpleBeanInfo; +import java.lang.ref.Reference; +import java.lang.ref.WeakReference; +import java.net.URL; +import java.net.URLClassLoader; + +public class Test5102804 { + private static final String BEAN_NAME = "Test5102804$Example"; + private static final String BEAN_INFO_NAME = BEAN_NAME + "BeanInfo"; + + public static void main(String[] args) { + if (!isCollectible(getReference())) + throw new Error("Reference is not collected"); + } + + private static Reference getReference() { + try { + ClassLoader loader = new Loader(); + Class type = Class.forName(BEAN_NAME, true, loader); + if (!type.getClassLoader().equals(loader)) { + throw new Error("Wrong class loader"); + } + BeanInfo info = Introspector.getBeanInfo(type); + if (0 != info.getDefaultPropertyIndex()) { + throw new Error("Wrong bean info found"); + } + return new WeakReference(type); + } + catch (IntrospectionException exception) { + throw new Error("Introspection Error", exception); + } + catch (ClassNotFoundException exception) { + throw new Error("Class Not Found", exception); + } + } + + private static boolean isCollectible(Reference reference) { + int[] array = new int[10]; + while (true) { + try { + array = new int[array.length + array.length / 3]; + } + catch (OutOfMemoryError error) { + return null == reference.get(); + } + } + } + + /** + * Custom class loader to load the Example class by itself. + * Could also load it from a different code source, but this is easier to set up. + */ + private static final class Loader extends URLClassLoader { + Loader() { + super(new URL[] { + Test5102804.class.getProtectionDomain().getCodeSource().getLocation() + }); + } + + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + Class c = findLoadedClass(name); + if (c == null) { + if (BEAN_NAME.equals(name) || BEAN_INFO_NAME.equals(name)) { + c = findClass(name); + } + else try { + c = getParent().loadClass(name); + } + catch (ClassNotFoundException exception) { + c = findClass(name); + } + } + if (resolve) { + resolveClass(c); + } + return c; + } + } + + /** + * A simple bean to load from the Loader class, not main class loader. + */ + public static final class Example { + private int value; + + public int getValue() { + return value; + } + + public void setValue(int value) { + this.value = value; + } + } + + /** + * The BeanInfo for the Example class. + * It is also loaded from the Loader class. + */ + public static final class ExampleBeanInfo extends SimpleBeanInfo { + @Override + public int getDefaultPropertyIndex() { + return 0; + } + + @Override + public PropertyDescriptor[] getPropertyDescriptors() { + try { + return new PropertyDescriptor[] { + new PropertyDescriptor("value", Class.forName(BEAN_NAME)) + }; + } + catch (ClassNotFoundException exception) { + return null; + } + catch (IntrospectionException exception) { + return null; + } + } + } +}