5102804: Memory leak in Introspector.getBeanInfo(Class) for custom BeanInfo: Class param

Reviewed-by: peterz

src/share/classes/com/sun/beans/WeakCache.java

@@ -81,4 +81,11 @@ public final class WeakCache<K, V> {
             this.map.remove(key);
         }
     }
+
+    /**
+     * Removes all of the mappings from this cache.
+     */
+    public void clear() {
+        this.map.clear();
+    }
 }

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<Class<?>, Method[]> declaredMethodCache =
+            new WeakCache<Class<?>, 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<Class<?>, BeanInfo> map;
         synchronized (BEANINFO_CACHE) {
-            map = (Map<Class<?>, BeanInfo>) AppContext.getAppContext().get(BEANINFO_CACHE);
-            if (map == null) {
-                map = Collections.synchronizedMap(new WeakHashMap<Class<?>, BeanInfo>());
-                AppContext.getAppContext().put(BEANINFO_CACHE, map);
+            WeakCache<Class<?>, BeanInfo> beanInfoCache =
+                    (WeakCache<Class<?>, BeanInfo>) AppContext.getAppContext().get(BEANINFO_CACHE);
+
+            if (beanInfoCache == null) {
+                beanInfoCache = new WeakCache<Class<?>, 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;
     }
 
     //======================================================================

test/java/beans/Introspector/Test5102804.java

+/*
+ * 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<Class>(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;
+            }
+        }
+    }
+}