8054987: (reflect) Add sharing of annotations between instances of Executable

Reviewed-by: duke

src/share/classes/java/lang/reflect/Constructor.java

@@ -94,8 +94,19 @@ public final class Constructor<T> extends Executable {
     // For sharing of ConstructorAccessors. This branching structure
     // is currently only two levels deep (i.e., one root Constructor
     // and potentially many Constructor objects pointing to it.)
+    //
+    // If this branching structure would ever contain cycles, deadlocks can
+    // occur in annotation code.
     private Constructor<T>      root;
 
+    /**
+     * Used by Excecutable for annotation sharing.
+     */
+    @Override
+    Executable getRoot() {
+        return root;
+    }
+
     /**
      * Package-private constructor used by ReflectAccess to enable
      * instantiation of these objects in Java code from the java.lang
@@ -132,6 +143,9 @@ public final class Constructor<T> extends Executable {
         // which implicitly requires that new java.lang.reflect
         // objects be fabricated for each reflective call on Class
         // objects.)
+        if (this.root != null)
+            throw new IllegalArgumentException("Can not copy a non-root Constructor");
+
         Constructor<T> res = new Constructor<>(clazz,
                                                parameterTypes,
                                                exceptionTypes, modifiers, slot,

src/share/classes/java/lang/reflect/Executable.java

@@ -52,6 +52,11 @@ public abstract class Executable extends AccessibleObject
      */
     abstract byte[] getAnnotationBytes();
 
+    /**
+     * Accessor method to allow code sharing
+     */
+    abstract Executable getRoot();
+
     /**
      * Does the Executable have generic information.
      */
@@ -543,11 +548,16 @@ public abstract class Executable extends AccessibleObject
 
     private synchronized  Map<Class<? extends Annotation>, Annotation> declaredAnnotations() {
         if (declaredAnnotations == null) {
-            declaredAnnotations = AnnotationParser.parseAnnotations(
-                getAnnotationBytes(),
-                sun.misc.SharedSecrets.getJavaLangAccess().
-                getConstantPool(getDeclaringClass()),
-                getDeclaringClass());
+            Executable root = getRoot();
+            if (root != null) {
+                declaredAnnotations = root.declaredAnnotations();
+            } else {
+                declaredAnnotations = AnnotationParser.parseAnnotations(
+                    getAnnotationBytes(),
+                    sun.misc.SharedSecrets.getJavaLangAccess().
+                    getConstantPool(getDeclaringClass()),
+                    getDeclaringClass());
+            }
         }
         return declaredAnnotations;
     }

src/share/classes/java/lang/reflect/Field.java

@@ -81,6 +81,9 @@ class Field extends AccessibleObject implements Member {
     // For sharing of FieldAccessors. This branching structure is
     // currently only two levels deep (i.e., one root Field and
     // potentially many Field objects pointing to it.)
+    //
+    // If this branching structure would ever contain cycles, deadlocks can
+    // occur in annotation code.
     private Field               root;
 
     // Generics infrastructure
@@ -141,6 +144,9 @@ class Field extends AccessibleObject implements Member {
         // which implicitly requires that new java.lang.reflect
         // objects be fabricated for each reflective call on Class
         // objects.)
+        if (this.root != null)
+            throw new IllegalArgumentException("Can not copy a non-root Field");
+
         Field res = new Field(clazz, name, type, modifiers, slot, signature, annotations);
         res.root = this;
         // Might as well eagerly propagate this if already present
@@ -1137,10 +1143,15 @@ class Field extends AccessibleObject implements Member {
 
     private synchronized  Map<Class<? extends Annotation>, Annotation> declaredAnnotations() {
         if (declaredAnnotations == null) {
-            declaredAnnotations = AnnotationParser.parseAnnotations(
-                annotations, sun.misc.SharedSecrets.getJavaLangAccess().
-                getConstantPool(getDeclaringClass()),
-                getDeclaringClass());
+            Field root = this.root;
+            if (root != null) {
+                declaredAnnotations = root.declaredAnnotations();
+            } else {
+                declaredAnnotations = AnnotationParser.parseAnnotations(
+                        annotations,
+                        sun.misc.SharedSecrets.getJavaLangAccess().getConstantPool(getDeclaringClass()),
+                        getDeclaringClass());
+            }
         }
         return declaredAnnotations;
     }

src/share/classes/java/lang/reflect/Method.java

@@ -79,6 +79,9 @@ public final class Method extends Executable {
     // For sharing of MethodAccessors. This branching structure is
     // currently only two levels deep (i.e., one root Method and
     // potentially many Method objects pointing to it.)
+    //
+    // If this branching structure would ever contain cycles, deadlocks can
+    // occur in annotation code.
     private Method              root;
 
     // Generics infrastructure
@@ -144,6 +147,9 @@ public final class Method extends Executable {
         // which implicitly requires that new java.lang.reflect
         // objects be fabricated for each reflective call on Class
         // objects.)
+        if (this.root != null)
+            throw new IllegalArgumentException("Can not copy a non-root Method");
+
         Method res = new Method(clazz, name, parameterTypes, returnType,
                                 exceptionTypes, modifiers, slot, signature,
                                 annotations, parameterAnnotations, annotationDefault);
@@ -153,6 +159,14 @@ public final class Method extends Executable {
         return res;
     }
 
+    /**
+     * Used by Excecutable for annotation sharing.
+     */
+    @Override
+    Executable getRoot() {
+        return root;
+    }
+
     @Override
     boolean hasGenericInformation() {
         return (getGenericSignature() != null);

test/java/lang/reflect/annotationSharing/AnnotationSharing.java

+/*
+ * Copyright (c) 2014, 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.
+ */
+
+/*
+ * @test
+ * @bug 8054987
+ * @summary Test sharing of annotations between Executable/Field instances.
+ *          Sharing should not be noticeable when performing mutating
+ *          operations.
+ * @run testng AnnotationSharing
+ */
+
+import java.lang.annotation.*;
+import java.lang.reflect.*;
+
+import org.testng.annotations.Test;
+
+public class AnnotationSharing {
+    public static void main(String ... args) throws Exception {
+    }
+
+    @Test
+    public void testMethodSharing() throws Exception {
+        Method[] m1 = AnnotationSharing.class.getMethods();
+        Method[] m2 = AnnotationSharing.class.getMethods();
+        validateSharingSafelyObservable(m1, m2);
+    }
+
+    @Test
+    public void testDeclaredMethodSharing() throws Exception {
+        Method[] m3 = AnnotationSharing.class.getDeclaredMethods();
+        Method[] m4 = AnnotationSharing.class.getDeclaredMethods();
+        validateSharingSafelyObservable(m3, m4);
+    }
+
+    @Test
+    public void testFieldSharing() throws Exception {
+        Field[] f1 = AnnotationSharing.class.getFields();
+        Field[] f2 = AnnotationSharing.class.getFields();
+        validateSharingSafelyObservable(f1, f2);
+    }
+
+    @Test
+    public void testDeclaredFieldsSharing() throws Exception {
+        Field[] f3 = AnnotationSharing.class.getDeclaredFields();
+        Field[] f4 = AnnotationSharing.class.getDeclaredFields();
+        validateSharingSafelyObservable(f3, f4);
+    }
+
+    @Test
+    public void testMethodSharingOccurs() throws Exception {
+        Method mm1 = AnnotationSharing.class.getDeclaredMethod("m", (Class<?>[])null);
+        Method mm2 = AnnotationSharing.class.getDeclaredMethod("m", (Class<?>[])null);
+        validateAnnotationSharing(mm1, mm2);
+    }
+
+    @Test
+    public void testMethodSharingIsSafe() throws Exception {
+        Method mm1 = AnnotationSharing.class.getDeclaredMethod("m", (Class<?>[])null);
+        Method mm2 = AnnotationSharing.class.getDeclaredMethod("m", (Class<?>[])null);
+        validateAnnotationSharingIsSafe(mm1, mm2);
+        validateArrayValues(mm1.getAnnotation(Baz.class), mm2.getAnnotation(Baz.class));
+    }
+
+    @Test
+    public void testFieldSharingOccurs() throws Exception {
+        Field ff1 = AnnotationSharing.class.getDeclaredField("f");
+        Field ff2 = AnnotationSharing.class.getDeclaredField("f");
+        validateAnnotationSharing(ff1, ff2);
+    }
+
+    @Test
+    public void testFieldSharingIsSafe() throws Exception {
+        Field ff1 = AnnotationSharing.class.getDeclaredField("f");
+        Field ff2 = AnnotationSharing.class.getDeclaredField("f");
+        validateAnnotationSharingIsSafe(ff1, ff2);
+        validateArrayValues(ff1.getAnnotation(Baz.class), ff2.getAnnotation(Baz.class));
+    }
+
+    // Validate that AccessibleObject instances are not shared
+    private static void validateSharingSafelyObservable(AccessibleObject[] m1, AccessibleObject[] m2)
+            throws Exception {
+
+        // Validate that setAccessible works
+        for (AccessibleObject m : m1)
+            m.setAccessible(false);
+
+        for (AccessibleObject m : m2)
+            m.setAccessible(true);
+
+        for (AccessibleObject m : m1)
+            if (m.isAccessible())
+                throw new RuntimeException(m + " should not be accessible");
+
+        for (AccessibleObject m : m2)
+            if (!m.isAccessible())
+                throw new RuntimeException(m + " should be accessible");
+
+        // Validate that methods are still equal()
+        for (int i = 0; i < m1.length; i++)
+            if (!m1[i].equals(m2[i]))
+                throw new RuntimeException(m1[i] + " and " + m2[i] + " should be equal()");
+
+        // Validate that the arrays aren't shared
+        for (int i = 0; i < m1.length; i++)
+            m1[i] = null;
+
+        for (int i = 0; i < m2.length; i++)
+            if (m2[i] == null)
+                throw new RuntimeException("Detected sharing of AccessibleObject arrays");
+    }
+
+    // Validate that annotations are shared
+    private static void validateAnnotationSharing(AccessibleObject m1, AccessibleObject m2) {
+        Bar b1 = m1.getAnnotation(Bar.class);
+        Bar b2 = m2.getAnnotation(Bar.class);
+
+        if (b1 != b2)
+            throw new RuntimeException(b1 + " and " + b2 + " should be ==");
+
+    }
+
+    // Validate that Method instances representing the annotation elements
+    // behave as intended
+    private static void validateAnnotationSharingIsSafe(AccessibleObject m1, AccessibleObject m2)
+            throws Exception {
+        Bar b1 = m1.getAnnotation(Bar.class);
+        Bar b2 = m2.getAnnotation(Bar.class);
+
+        Method mm1 = b1.annotationType().getMethod("value", (Class<?>[]) null);
+        Method mm2 = b2.annotationType().getMethod("value", (Class<?>[]) null);
+        inner(mm1, mm2);
+
+        mm1 = b1.getClass().getMethod("value", (Class<?>[]) null);
+        mm2 = b2.getClass().getMethod("value", (Class<?>[]) null);
+        inner(mm1, mm2);
+
+    }
+    private static void inner(Method mm1, Method mm2)
+            throws Exception {
+        if (!mm1.equals(mm2))
+            throw new RuntimeException(mm1 + " and " + mm2 + " should be equal()");
+
+        mm1.setAccessible(false);
+        mm2.setAccessible(true);
+
+        if (mm1.isAccessible())
+            throw new RuntimeException(mm1 + " should not be accessible");
+
+        if (!mm2.isAccessible())
+            throw new RuntimeException(mm2 + " should be accessible");
+    }
+
+    // Validate that array element values are not shared
+    private static void validateArrayValues(Baz a, Baz b) {
+        String[] s1 = a.value();
+        String[] s2 = b.value();
+
+        s1[0] = "22";
+
+        if (!s2[0].equals("1"))
+            throw new RuntimeException("Mutation of array elements should not be detectable");
+    }
+
+    @Foo @Bar("val") @Baz({"1", "2"})
+    public void m() {
+        return ;
+    }
+
+    @Foo @Bar("someValue") @Baz({"1", "22", "33"})
+    public Object f = new Object();
+}
+
+@Retention(RetentionPolicy.RUNTIME)
+@interface Foo {}
+
+@Retention(RetentionPolicy.RUNTIME)
+@interface Bar {
+    String value();
+}
+
+@Retention(RetentionPolicy.RUNTIME)
+@interface Baz {
+    String [] value();
+}