# HG changeset patch # User plevart # Date 1373878553 -7200 # Node ID e4ce6502eac0277c8619b82eada6c6852ff8a0f5 # Parent 5f2a8db78aca1286b1f70be3249b2d7d59e3d49b 7122142: (ann) Race condition between isAnnotationPresent and getAnnotations Reviewed-by: dholmes, jfranck --- a/src/share/classes/java/lang/Class.java Sat Jul 13 08:47:49 2013 +0800 +++ b/src/share/classes/java/lang/Class.java Mon Jul 15 10:55:53 2013 +0200 @@ -2308,6 +2308,45 @@ public final class Class implements j } /** + * Atomic operations support. + */ + private static class Atomic { + // initialize Unsafe machinery here, since we need to call Class.class instance method + // and have to avoid calling it in the static initializer of the Class class... + private static final Unsafe unsafe = Unsafe.getUnsafe(); + // offset of Class.reflectionData instance field + private static final long reflectionDataOffset; + // offset of Class.annotationType instance field + private static final long annotationTypeOffset; + + static { + Field[] fields = Class.class.getDeclaredFields0(false); // bypass caches + reflectionDataOffset = objectFieldOffset(fields, "reflectionData"); + annotationTypeOffset = objectFieldOffset(fields, "annotationType"); + } + + private static long objectFieldOffset(Field[] fields, String fieldName) { + Field field = searchFields(fields, fieldName); + if (field == null) { + throw new Error("No " + fieldName + " field found in java.lang.Class"); + } + return unsafe.objectFieldOffset(field); + } + + static boolean casReflectionData(Class clazz, + SoftReference> oldData, + SoftReference> newData) { + return unsafe.compareAndSwapObject(clazz, reflectionDataOffset, oldData, newData); + } + + static boolean casAnnotationType(Class clazz, + AnnotationType oldType, + AnnotationType newType) { + return unsafe.compareAndSwapObject(clazz, annotationTypeOffset, oldType, newType); + } + } + + /** * Reflection support. */ @@ -2333,29 +2372,6 @@ public final class Class implements j ReflectionData(int redefinedCount) { this.redefinedCount = redefinedCount; } - - // initialize Unsafe machinery here, since we need to call Class.class instance method - // and have to avoid calling it in the static initializer of the Class class... - private static final Unsafe unsafe; - // offset of Class.reflectionData instance field - private static final long reflectionDataOffset; - - static { - unsafe = Unsafe.getUnsafe(); - // bypass caches - Field reflectionDataField = searchFields(Class.class.getDeclaredFields0(false), - "reflectionData"); - if (reflectionDataField == null) { - throw new Error("No reflectionData field found in java.lang.Class"); - } - reflectionDataOffset = unsafe.objectFieldOffset(reflectionDataField); - } - - static boolean compareAndSwap(Class clazz, - SoftReference> oldData, - SoftReference> newData) { - return unsafe.compareAndSwapObject(clazz, reflectionDataOffset, oldData, newData); - } } private volatile transient SoftReference> reflectionData; @@ -2387,7 +2403,7 @@ public final class Class implements j while (true) { ReflectionData rd = new ReflectionData<>(classRedefinedCount); // try to CAS it... - if (ReflectionData.compareAndSwap(this, oldReflectionData, new SoftReference<>(rd))) { + if (Atomic.casReflectionData(this, oldReflectionData, new SoftReference<>(rd))) { return rd; } // else retry @@ -2430,7 +2446,7 @@ public final class Class implements j } // Annotations handling - private native byte[] getRawAnnotations(); + native byte[] getRawAnnotations(); // Since 1.8 native byte[] getRawTypeAnnotations(); static byte[] getExecutableTypeAnnotationBytes(Executable ex) { @@ -3290,10 +3306,11 @@ public final class Class implements j // Annotation types cache their internal (AnnotationType) form - private AnnotationType annotationType; - - void setAnnotationType(AnnotationType type) { - annotationType = type; + @SuppressWarnings("UnusedDeclaration") + private volatile transient AnnotationType annotationType; + + boolean casAnnotationType(AnnotationType oldType, AnnotationType newType) { + return Atomic.casAnnotationType(this, oldType, newType); } AnnotationType getAnnotationType() { --- a/src/share/classes/java/lang/System.java Sat Jul 13 08:47:49 2013 +0800 +++ b/src/share/classes/java/lang/System.java Mon Jul 15 10:55:53 2013 +0200 @@ -1220,11 +1220,14 @@ public final class System { public sun.reflect.ConstantPool getConstantPool(Class klass) { return klass.getConstantPool(); } - public void setAnnotationType(Class klass, AnnotationType type) { - klass.setAnnotationType(type); + public boolean casAnnotationType(Class klass, AnnotationType oldType, AnnotationType newType) { + return klass.casAnnotationType(oldType, newType); } public AnnotationType getAnnotationType(Class klass) { return klass.getAnnotationType(); + } + public byte[] getRawClassAnnotations(Class klass) { + return klass.getRawAnnotations(); } public byte[] getRawClassTypeAnnotations(Class klass) { return klass.getRawTypeAnnotations(); --- a/src/share/classes/sun/misc/JavaLangAccess.java Sat Jul 13 08:47:49 2013 +0800 +++ b/src/share/classes/sun/misc/JavaLangAccess.java Mon Jul 15 10:55:53 2013 +0200 @@ -36,16 +36,22 @@ public interface JavaLangAccess { ConstantPool getConstantPool(Class klass); /** - * Set the AnnotationType instance corresponding to this class. + * Compare-And-Swap the AnnotationType instance corresponding to this class. * (This method only applies to annotation types.) */ - void setAnnotationType(Class klass, AnnotationType annotationType); + boolean casAnnotationType(Class klass, AnnotationType oldType, AnnotationType newType); /** * Get the AnnotationType instance corresponding to this class. * (This method only applies to annotation types.) */ AnnotationType getAnnotationType(Class klass); + + /** + * Get the array of bytes that is the class-file representation + * of this Class' annotations. + */ + byte[] getRawClassAnnotations(Class klass); /** * Get the array of bytes that is the class-file representation --- a/src/share/classes/sun/reflect/annotation/AnnotationParser.java Sat Jul 13 08:47:49 2013 +0800 +++ b/src/share/classes/sun/reflect/annotation/AnnotationParser.java Mon Jul 15 10:55:53 2013 +0200 @@ -69,7 +69,7 @@ public class AnnotationParser { return Collections.emptyMap(); try { - return parseAnnotations2(rawAnnotations, constPool, container); + return parseAnnotations2(rawAnnotations, constPool, container, null); } catch(BufferUnderflowException e) { throw new AnnotationFormatError("Unexpected end of annotations."); } catch(IllegalArgumentException e) { @@ -78,24 +78,53 @@ public class AnnotationParser { } } + /** + * Like {@link #parseAnnotations(byte[], sun.reflect.ConstantPool, Class)} + * with an additional parameter {@code selectAnnotationClasses} which selects the + * annotation types to parse (other than selected are quickly skipped).

+ * This method is only used to parse select meta annotations in the construction + * phase of {@link AnnotationType} instances to prevent infinite recursion. + * + * @param selectAnnotationClasses an array of annotation types to select when parsing + */ + @SafeVarargs + static Map, Annotation> parseSelectAnnotations( + byte[] rawAnnotations, + ConstantPool constPool, + Class container, + Class ... selectAnnotationClasses) { + if (rawAnnotations == null) + return Collections.emptyMap(); + + try { + return parseAnnotations2(rawAnnotations, constPool, container, selectAnnotationClasses); + } catch(BufferUnderflowException e) { + throw new AnnotationFormatError("Unexpected end of annotations."); + } catch(IllegalArgumentException e) { + // Type mismatch in constant pool + throw new AnnotationFormatError(e); + } + } + private static Map, Annotation> parseAnnotations2( byte[] rawAnnotations, ConstantPool constPool, - Class container) { + Class container, + Class[] selectAnnotationClasses) { Map, Annotation> result = new LinkedHashMap, Annotation>(); ByteBuffer buf = ByteBuffer.wrap(rawAnnotations); int numAnnotations = buf.getShort() & 0xFFFF; for (int i = 0; i < numAnnotations; i++) { - Annotation a = parseAnnotation(buf, constPool, container, false); + Annotation a = parseAnnotation2(buf, constPool, container, false, selectAnnotationClasses); if (a != null) { Class klass = a.annotationType(); - AnnotationType type = AnnotationType.getInstance(klass); - if (type.retention() == RetentionPolicy.RUNTIME) - if (result.put(klass, a) != null) + if (AnnotationType.getInstance(klass).retention() == RetentionPolicy.RUNTIME && + result.put(klass, a) != null) { throw new AnnotationFormatError( "Duplicate annotation for class: "+klass+": " + a); } + } } return result; } @@ -189,11 +218,19 @@ public class AnnotationParser { * TypeNotPresentException if a referenced annotation type is not * available at runtime */ - @SuppressWarnings("unchecked") static Annotation parseAnnotation(ByteBuffer buf, ConstantPool constPool, Class container, boolean exceptionOnMissingAnnotationClass) { + return parseAnnotation2(buf, constPool, container, exceptionOnMissingAnnotationClass, null); + } + + @SuppressWarnings("unchecked") + private static Annotation parseAnnotation2(ByteBuffer buf, + ConstantPool constPool, + Class container, + boolean exceptionOnMissingAnnotationClass, + Class[] selectAnnotationClasses) { int typeIndex = buf.getShort() & 0xFFFF; Class annotationClass = null; String sig = "[unknown]"; @@ -216,6 +253,10 @@ public class AnnotationParser { catch (TypeNotPresentException e) { if (exceptionOnMissingAnnotationClass) throw e; + skipAnnotation(buf, false); + return null; + } + if (selectAnnotationClasses != null && !contains(selectAnnotationClasses, annotationClass)) { skipAnnotation(buf, false); return null; } @@ -800,6 +841,17 @@ public class AnnotationParser { skipMemberValue(buf); } + /** + * Searches for given {@code element} in given {@code array} by identity. + * Returns {@code true} if found {@code false} if not. + */ + private static boolean contains(Object[] array, Object element) { + for (Object e : array) + if (e == element) + return true; + return false; + } + /* * This method converts the annotation map returned by the parseAnnotations() * method to an array. It is called by Field.getDeclaredAnnotations(), --- a/src/share/classes/sun/reflect/annotation/AnnotationType.java Sat Jul 13 08:47:49 2013 +0800 +++ b/src/share/classes/sun/reflect/annotation/AnnotationType.java Mon Jul 15 10:55:53 2013 +0200 @@ -25,6 +25,8 @@ package sun.reflect.annotation; +import sun.misc.JavaLangAccess; + import java.lang.annotation.*; import java.lang.reflect.*; import java.util.*; @@ -61,12 +63,12 @@ public class AnnotationType { /** * The retention policy for this annotation type. */ - private RetentionPolicy retention = RetentionPolicy.RUNTIME;; + private final RetentionPolicy retention; /** * Whether this annotation type is inherited. */ - private boolean inherited = false; + private final boolean inherited; /** * Returns an AnnotationType instance for the specified annotation type. @@ -74,13 +76,20 @@ public class AnnotationType { * @throw IllegalArgumentException if the specified class object for * does not represent a valid annotation type */ - public static synchronized AnnotationType getInstance( + public static AnnotationType getInstance( Class annotationClass) { - AnnotationType result = sun.misc.SharedSecrets.getJavaLangAccess(). - getAnnotationType(annotationClass); - if (result == null) - result = new AnnotationType((Class) annotationClass); + JavaLangAccess jla = sun.misc.SharedSecrets.getJavaLangAccess(); + AnnotationType result = jla.getAnnotationType(annotationClass); // volatile read + if (result == null) { + result = new AnnotationType(annotationClass); + // try to CAS the AnnotationType: null -> result + if (!jla.casAnnotationType(annotationClass, null, result)) { + // somebody was quicker -> read it's result + result = jla.getAnnotationType(annotationClass); + assert result != null; + } + } return result; } @@ -121,16 +130,25 @@ public class AnnotationType { memberDefaults.put(name, defaultValue); } - sun.misc.SharedSecrets.getJavaLangAccess(). - setAnnotationType(annotationClass, this); - // Initialize retention, & inherited fields. Special treatment // of the corresponding annotation types breaks infinite recursion. if (annotationClass != Retention.class && annotationClass != Inherited.class) { - Retention ret = annotationClass.getAnnotation(Retention.class); + JavaLangAccess jla = sun.misc.SharedSecrets.getJavaLangAccess(); + Map, Annotation> metaAnnotations = + AnnotationParser.parseSelectAnnotations( + jla.getRawClassAnnotations(annotationClass), + jla.getConstantPool(annotationClass), + annotationClass, + Retention.class, Inherited.class + ); + Retention ret = (Retention) metaAnnotations.get(Retention.class); retention = (ret == null ? RetentionPolicy.CLASS : ret.value()); - inherited = annotationClass.isAnnotationPresent(Inherited.class); + inherited = metaAnnotations.containsKey(Inherited.class); + } + else { + retention = RetentionPolicy.RUNTIME; + inherited = false; } } @@ -205,11 +223,10 @@ public class AnnotationType { * For debugging. */ public String toString() { - StringBuffer s = new StringBuffer("Annotation Type:" + "\n"); - s.append(" Member types: " + memberTypes + "\n"); - s.append(" Member defaults: " + memberDefaults + "\n"); - s.append(" Retention policy: " + retention + "\n"); - s.append(" Inherited: " + inherited); - return s.toString(); + return "Annotation Type:\n" + + " Member types: " + memberTypes + "\n" + + " Member defaults: " + memberDefaults + "\n" + + " Retention policy: " + retention + "\n" + + " Inherited: " + inherited; } } --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/java/lang/annotation/AnnotationType/AnnotationTypeDeadlockTest.java Mon Jul 15 10:55:53 2013 +0200 @@ -0,0 +1,105 @@ +/* + * 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. + */ + +/* + * @test + * @bug 7122142 + * @summary Test deadlock situation when recursive annotations are parsed + */ + +import java.lang.annotation.Retention; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicInteger; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +public class AnnotationTypeDeadlockTest { + + @Retention(RUNTIME) + @AnnB + public @interface AnnA { + } + + @Retention(RUNTIME) + @AnnA + public @interface AnnB { + } + + static class Task extends Thread { + final CountDownLatch prepareLatch; + final AtomicInteger goLatch; + final Class clazz; + + Task(CountDownLatch prepareLatch, AtomicInteger goLatch, Class clazz) { + super(clazz.getSimpleName()); + setDaemon(true); // in case it deadlocks + this.prepareLatch = prepareLatch; + this.goLatch = goLatch; + this.clazz = clazz; + } + + @Override + public void run() { + prepareLatch.countDown(); // notify we are prepared + while (goLatch.get() > 0); // spin-wait before go + clazz.getDeclaredAnnotations(); + } + } + + static void dumpState(Task task) { + System.err.println( + "Task[" + task.getName() + "].state: " + + task.getState() + " ..." + ); + for (StackTraceElement ste : task.getStackTrace()) { + System.err.println("\tat " + ste); + } + System.err.println(); + } + + public static void main(String[] args) throws Exception { + CountDownLatch prepareLatch = new CountDownLatch(2); + AtomicInteger goLatch = new AtomicInteger(1); + Task taskA = new Task(prepareLatch, goLatch, AnnA.class); + Task taskB = new Task(prepareLatch, goLatch, AnnB.class); + taskA.start(); + taskB.start(); + // wait until both threads start-up + prepareLatch.await(); + // let them go + goLatch.set(0); + // attempt to join them + taskA.join(5000L); + taskB.join(5000L); + + if (taskA.isAlive() || taskB.isAlive()) { + dumpState(taskA); + dumpState(taskB); + throw new IllegalStateException( + taskA.getState() == Thread.State.BLOCKED && + taskB.getState() == Thread.State.BLOCKED + ? "deadlock detected" + : "unexpected condition"); + } + } +} --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/java/lang/annotation/AnnotationType/AnnotationTypeRuntimeAssumptionTest.java Mon Jul 15 10:55:53 2013 +0200 @@ -0,0 +1,176 @@ +/* + * 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. + */ + +/* + * @test + * @summary Test consistent parsing of ex-RUNTIME annotations that + * were changed and separately compiled to have CLASS retention + */ + +import sun.misc.IOUtils; + +import java.io.IOException; +import java.io.InputStream; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +import static java.lang.annotation.RetentionPolicy.CLASS; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +/** + * This test simulates a situation where there are two mutually recursive + * {@link RetentionPolicy#RUNTIME RUNTIME} annotations {@link AnnA_v1 AnnA_v1} + * and {@link AnnB AnnB} and then the first is changed to have + * {@link RetentionPolicy#CLASS CLASS} retention and separately compiled. + * When {@link AnnA_v1 AnnA_v1} annotation is looked-up on {@link AnnB AnnB} + * it still appears to have {@link RetentionPolicy#RUNTIME RUNTIME} retention. + */ +public class AnnotationTypeRuntimeAssumptionTest { + + @Retention(RUNTIME) + @AnnB + public @interface AnnA_v1 { + } + + // An alternative version of AnnA_v1 with CLASS retention instead. + // Used to simulate separate compilation (see AltClassLoader below). + @Retention(CLASS) + @AnnB + public @interface AnnA_v2 { + } + + @Retention(RUNTIME) + @AnnA_v1 + public @interface AnnB { + } + + @AnnA_v1 + public static class TestTask implements Runnable { + @Override + public void run() { + AnnA_v1 ann1 = TestTask.class.getDeclaredAnnotation(AnnA_v1.class); + if (ann1 != null) { + throw new IllegalStateException( + "@" + ann1.annotationType().getSimpleName() + + " found on: " + TestTask.class.getName() + + " should not be visible at runtime"); + } + AnnA_v1 ann2 = AnnB.class.getDeclaredAnnotation(AnnA_v1.class); + if (ann2 != null) { + throw new IllegalStateException( + "@" + ann2.annotationType().getSimpleName() + + " found on: " + AnnB.class.getName() + + " should not be visible at runtime"); + } + } + } + + public static void main(String[] args) throws Exception { + ClassLoader altLoader = new AltClassLoader( + AnnotationTypeRuntimeAssumptionTest.class.getClassLoader()); + + Runnable altTask = (Runnable) Class.forName( + TestTask.class.getName(), + true, + altLoader).newInstance(); + + altTask.run(); + } + + /** + * A ClassLoader implementation that loads alternative implementations of + * classes. If class name ends with "_v1" it locates instead a class with + * name ending with "_v2" and loads that class instead. + */ + static class AltClassLoader extends ClassLoader { + AltClassLoader(ClassLoader parent) { + super(parent); + } + + @Override + protected Class loadClass(String name, boolean resolve) + throws ClassNotFoundException { + if (name.indexOf('.') < 0) { // root package is our class + synchronized (getClassLoadingLock(name)) { + // First, check if the class has already been loaded + Class c = findLoadedClass(name); + if (c == null) { + c = findClass(name); + } + if (resolve) { + resolveClass(c); + } + return c; + } + } + else { // not our class + return super.loadClass(name, resolve); + } + } + + @Override + protected Class findClass(String name) + throws ClassNotFoundException { + // special class name -> replace it with alternative name + if (name.endsWith("_v1")) { + String altName = name.substring(0, name.length() - 3) + "_v2"; + String altPath = altName.replace('.', '/').concat(".class"); + try (InputStream is = getResourceAsStream(altPath)) { + if (is != null) { + byte[] bytes = IOUtils.readFully(is, -1, true); + // patch class bytes to contain original name + for (int i = 0; i < bytes.length - 2; i++) { + if (bytes[i] == '_' && + bytes[i + 1] == 'v' && + bytes[i + 2] == '2') { + bytes[i + 2] = '1'; + } + } + return defineClass(name, bytes, 0, bytes.length); + } + else { + throw new ClassNotFoundException(name); + } + } + catch (IOException e) { + throw new ClassNotFoundException(name, e); + } + } + else { // not special class name -> just load the class + String path = name.replace('.', '/').concat(".class"); + try (InputStream is = getResourceAsStream(path)) { + if (is != null) { + byte[] bytes = IOUtils.readFully(is, -1, true); + return defineClass(name, bytes, 0, bytes.length); + } + else { + throw new ClassNotFoundException(name); + } + } + catch (IOException e) { + throw new ClassNotFoundException(name, e); + } + } + } + } +}