# HG changeset patch # User jfranck # Date 1408348616 -7200 # Mon Aug 18 09:56:56 2014 +0200 # Node ID 95ce477ef0067d6272022768bfa11bdf0448aeca # Parent 6407a15e2274dc8490849c2e442fd0321f10dac5 8054987: (reflect) Add sharing of annotations between instances of Executable Reviewed-by: duke diff --git a/src/share/classes/java/lang/reflect/Constructor.java b/src/share/classes/java/lang/reflect/Constructor.java --- a/src/share/classes/java/lang/reflect/Constructor.java +++ b/src/share/classes/java/lang/reflect/Constructor.java @@ -94,9 +94,20 @@ // 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 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 * package via sun.reflect.LangReflectAccess. @@ -132,6 +143,9 @@ // 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 res = new Constructor<>(clazz, parameterTypes, exceptionTypes, modifiers, slot, diff --git a/src/share/classes/java/lang/reflect/Executable.java b/src/share/classes/java/lang/reflect/Executable.java --- a/src/share/classes/java/lang/reflect/Executable.java +++ b/src/share/classes/java/lang/reflect/Executable.java @@ -53,6 +53,11 @@ abstract byte[] getAnnotationBytes(); /** + * Accessor method to allow code sharing + */ + abstract Executable getRoot(); + + /** * Does the Executable have generic information. */ abstract boolean hasGenericInformation(); @@ -540,11 +545,16 @@ private synchronized Map, 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; } diff --git a/src/share/classes/java/lang/reflect/Field.java b/src/share/classes/java/lang/reflect/Field.java --- a/src/share/classes/java/lang/reflect/Field.java +++ b/src/share/classes/java/lang/reflect/Field.java @@ -81,6 +81,9 @@ // 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 @@ // 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 @@ private synchronized Map, 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; } diff --git a/src/share/classes/java/lang/reflect/Method.java b/src/share/classes/java/lang/reflect/Method.java --- a/src/share/classes/java/lang/reflect/Method.java +++ b/src/share/classes/java/lang/reflect/Method.java @@ -79,6 +79,9 @@ // 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 @@ // 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 @@ return res; } + /** + * Used by Excecutable for annotation sharing. + */ + @Override + Executable getRoot() { + return root; + } + @Override boolean hasGenericInformation() { return (getGenericSignature() != null); diff --git a/test/java/lang/reflect/annotationSharing/AnnotationSharing.java b/test/java/lang/reflect/annotationSharing/AnnotationSharing.java new file mode 100644 --- /dev/null +++ b/test/java/lang/reflect/annotationSharing/AnnotationSharing.java @@ -0,0 +1,205 @@ +/* + * 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(); +}