8028054: com.sun.beans.finder.MethodFinder has unsynchronized access to a static Map

Reviewed-by: alexsch, serb

src/share/classes/com/sun/beans/finder/ConstructorFinder.java

@@ -24,11 +24,12 @@
  */
 package com.sun.beans.finder;
 
-import com.sun.beans.WeakCache;
+import com.sun.beans.util.Cache;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Modifier;
 
+import static com.sun.beans.util.Cache.Kind.SOFT;
 import static sun.reflect.misc.ReflectUtil.isPackageAccessible;
 
 /**
@@ -41,7 +42,18 @@ import static sun.reflect.misc.ReflectUtil.isPackageAccessible;
  * @author Sergey A. Malenkov
  */
 public final class ConstructorFinder extends AbstractFinder<Constructor<?>> {
-    private static final WeakCache<Signature, Constructor<?>> CACHE = new WeakCache<Signature, Constructor<?>>();
+    private static final Cache<Signature, Constructor<?>> CACHE = new Cache<Signature, Constructor<?>>(SOFT, SOFT) {
+        @Override
+        public Constructor create(Signature signature) {
+            try {
+                ConstructorFinder finder = new ConstructorFinder(signature.getArgs());
+                return finder.find(signature.getType().getConstructors());
+            }
+            catch (Exception exception) {
+                throw new SignatureException(exception);
+            }
+        }
+    };
 
     /**
      * Finds public constructor
@@ -69,13 +81,12 @@ public final class ConstructorFinder extends AbstractFinder<Constructor<?>> {
         PrimitiveWrapperMap.replacePrimitivesWithWrappers(args);
         Signature signature = new Signature(type, args);
 
-        Constructor<?> constructor = CACHE.get(signature);
-        if (constructor != null) {
-            return constructor;
+        try {
+            return CACHE.get(signature);
+        }
+        catch (SignatureException exception) {
+            throw exception.toNoSuchMethodException("Constructor is not found");
         }
-        constructor = new ConstructorFinder(args).find(type.getConstructors());
-        CACHE.put(signature, constructor);
-        return constructor;
     }
 
     /**

src/share/classes/com/sun/beans/finder/MethodFinder.java

@@ -25,7 +25,7 @@
 package com.sun.beans.finder;
 
 import com.sun.beans.TypeResolver;
-import com.sun.beans.WeakCache;
+import com.sun.beans.util.Cache;
 
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -33,6 +33,7 @@ import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.Arrays;
 
+import static com.sun.beans.util.Cache.Kind.SOFT;
 import static sun.reflect.misc.ReflectUtil.isPackageAccessible;
 
 /**
@@ -45,7 +46,18 @@ import static sun.reflect.misc.ReflectUtil.isPackageAccessible;
  * @author Sergey A. Malenkov
  */
 public final class MethodFinder extends AbstractFinder<Method> {
-    private static final WeakCache<Signature, Method> CACHE = new WeakCache<Signature, Method>();
+    private static final Cache<Signature, Method> CACHE = new Cache<Signature, Method>(SOFT, SOFT) {
+        @Override
+        public Method create(Signature signature) {
+            try {
+                MethodFinder finder = new MethodFinder(signature.getName(), signature.getArgs());
+                return findAccessibleMethod(finder.find(signature.getType().getMethods()));
+            }
+            catch (Exception exception) {
+                throw new SignatureException(exception);
+            }
+        }
+    };
 
     /**
      * Finds public method (static or non-static)
@@ -65,16 +77,13 @@ public final class MethodFinder extends AbstractFinder<Method> {
         PrimitiveWrapperMap.replacePrimitivesWithWrappers(args);
         Signature signature = new Signature(type, name, args);
 
-        Method method = CACHE.get(signature);
-        boolean cached = method != null;
-        if (cached && isPackageAccessible(method.getDeclaringClass())) {
-            return method;
+        try {
+            Method method = CACHE.get(signature);
+            return (method == null) || isPackageAccessible(method.getDeclaringClass()) ? method : CACHE.create(signature);
         }
-        method = findAccessibleMethod(new MethodFinder(name, args).find(type.getMethods()));
-        if (!cached) {
-            CACHE.put(signature, method);
+        catch (SignatureException exception) {
+            throw exception.toNoSuchMethodException("Method '" + name + "' is not found");
         }
-        return method;
     }
 
     /**

src/share/classes/com/sun/beans/finder/Signature.java

@@ -62,6 +62,18 @@ final class Signature {
         this.args = args;
     }
 
+    Class<?> getType() {
+        return this.type;
+    }
+
+    String getName() {
+        return this.name;
+    }
+
+    Class<?>[] getArgs() {
+        return this.args;
+    }
+
     /**
      * Indicates whether some other object is "equal to" this one.
      *

src/share/classes/com/sun/beans/finder/SignatureException.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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 Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.sun.beans.finder;
+
+final class SignatureException extends RuntimeException {
+    SignatureException(Throwable cause) {
+        super(cause);
+    }
+
+    NoSuchMethodException toNoSuchMethodException(String message) {
+        Throwable throwable = getCause();
+        if (throwable instanceof NoSuchMethodException) {
+            return (NoSuchMethodException) throwable;
+        }
+        NoSuchMethodException exception = new NoSuchMethodException(message);
+        exception.initCause(throwable);
+        return exception;
+    }
+}

test/java/beans/XMLDecoder/8028054/Task.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. 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 Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+abstract class Task<T> implements Runnable {
+    private transient boolean working = true;
+    private final List<T> methods;
+    private final Thread thread;
+
+    Task(List<T> methods) {
+        this.methods = methods;
+        this.thread = new Thread(this);
+        this.thread.start();
+    }
+
+    boolean isAlive() {
+        return this.thread.isAlive();
+    }
+
+    boolean isWorking() {
+        boolean working = this.working && this.thread.isAlive();
+        this.working = false;
+        return working;
+    }
+
+    @Override
+    public void run() {
+        long time = -System.currentTimeMillis();
+        for (T method : this.methods) {
+            this.working = true;
+            try {
+                for (int i = 0; i < 100; i++) {
+                    process(method);
+                }
+            } catch (NoSuchMethodException ignore) {
+            }
+        }
+        time += System.currentTimeMillis();
+        print("thread done in " + time / 1000 + " seconds");
+    }
+
+    protected abstract void process(T method) throws NoSuchMethodException;
+
+    static synchronized void print(Object message) {
+        System.out.println(message);
+        System.out.flush();
+    }
+
+    static List<Class<?>> getClasses(int count) throws Exception {
+        String resource = ClassLoader.getSystemClassLoader().getResource("java/lang/Object.class").toString();
+
+        Pattern pattern = Pattern.compile("jar:file:(.*)!.*");
+        Matcher matcher = pattern.matcher(resource);
+        matcher.matches();
+        resource = matcher.group(1);
+
+        List<Class<?>> classes = new ArrayList<>();
+        try (JarFile jarFile = new JarFile(resource)) {
+            Enumeration<JarEntry> entries = jarFile.entries();
+            while (entries.hasMoreElements()) {
+                String name = entries.nextElement().getName();
+                if (name.startsWith("java") && name.endsWith(".class")) {
+                    classes.add(Class.forName(name.substring(0, name.indexOf(".")).replace('/', '.')));
+                    if (count == classes.size()) {
+                        break;
+                    }
+                }
+            }
+        }
+        return classes;
+    }
+}

test/java/beans/XMLDecoder/8028054/TestConstructorFinder.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. 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 Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import com.sun.beans.finder.ConstructorFinder;
+
+import java.lang.reflect.Constructor;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/*
+ * @test
+ * @bug 8028054
+ * @summary Tests that cached constructors have synchronized access
+ * @author Sergey Malenkov
+ * @compile -XDignore.symbol.file TestConstructorFinder.java
+ * @run main TestConstructorFinder
+ */
+
+public class TestConstructorFinder {
+    public static void main(String[] args) throws Exception {
+        List<Class<?>> classes = Task.getClasses(Integer.MAX_VALUE);
+        List<Constructor> constructors = new ArrayList<>();
+        for (Class<?> type : classes) {
+            Collections.addAll(constructors, type.getConstructors());
+        }
+        Task.print("found " + constructors.size() + " constructors in " + classes.size() + " classes");
+
+        List<Task> tasks = new ArrayList<>();
+        for (int i = 0; i < 50; i++) {
+            tasks.add(new Task<Constructor>(constructors) {
+                @Override
+                protected void process(Constructor constructor) throws NoSuchMethodException {
+                    ConstructorFinder.findConstructor(constructor.getDeclaringClass(), constructor.getParameterTypes());
+                }
+            });
+        }
+        int alarm = 0;
+        while (true) {
+            int alive = 0;
+            int working = 0;
+            for (Task task : tasks) {
+                if (task.isWorking()) {
+                    working++;
+                    alive++;
+                } else if (task.isAlive()) {
+                    alive++;
+                }
+            }
+            if (alive == 0) {
+                break;
+            }
+            Task.print(working + " out of " + alive + " threads are working");
+            if ((working == 0) && (++alarm == 10)) {
+                Task.print("DEADLOCK DETECTED");
+                System.exit(100);
+            }
+            Thread.sleep(1000);
+        }
+    }
+}

test/java/beans/XMLDecoder/8028054/TestMethodFinder.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. 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 Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import com.sun.beans.finder.MethodFinder;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/*
+ * @test
+ * @bug 8028054
+ * @summary Tests that cached methods have synchronized access
+ * @author Sergey Malenkov
+ * @compile -XDignore.symbol.file TestMethodFinder.java
+ * @run main TestMethodFinder
+ */
+
+public class TestMethodFinder {
+    public static void main(String[] args) throws Exception {
+        List<Class<?>> classes = Task.getClasses(4000);
+        List<Method> methods = new ArrayList<>();
+        for (Class<?> type : classes) {
+            Collections.addAll(methods, type.getMethods());
+        }
+        Task.print("found " + methods.size() + " methods in " + classes.size() + " classes");
+
+        List<Task> tasks = new ArrayList<>();
+        for (int i = 0; i < 50; i++) {
+            tasks.add(new Task<Method>(methods) {
+                @Override
+                protected void process(Method method) throws NoSuchMethodException {
+                    MethodFinder.findMethod(method.getDeclaringClass(), method.getName(), method.getParameterTypes());
+                }
+            });
+        }
+        int alarm = 0;
+        while (true) {
+            int alive = 0;
+            int working = 0;
+            for (Task task : tasks) {
+                if (task.isWorking()) {
+                    working++;
+                    alive++;
+                } else if (task.isAlive()) {
+                    alive++;
+                }
+            }
+            if (alive == 0) {
+                break;
+            }
+            Task.print(working + " out of " + alive + " threads are working");
+            if ((working == 0) && (++alarm == 10)) {
+                Task.print("DEADLOCK DETECTED");
+                System.exit(100);
+            }
+            Thread.sleep(1000);
+        }
+    }
+}