Mercurial > hg > shenandoah-preopenjdk-archive > openjdk8 > jdk
changeset 10297:fc4f55464170 jdk8u40-b17
8062771: Core reflection should use final fields whenever possible
Reviewed-by: martin, jfranck
line wrap: on
line diff
--- a/src/share/classes/sun/reflect/BootstrapConstructorAccessorImpl.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/BootstrapConstructorAccessorImpl.java Fri Nov 28 16:17:30 2014 +0300 @@ -32,7 +32,7 @@ bootstrapping. */ class BootstrapConstructorAccessorImpl extends ConstructorAccessorImpl { - private Constructor<?> constructor; + private final Constructor<?> constructor; BootstrapConstructorAccessorImpl(Constructor<?> c) { this.constructor = c;
--- a/src/share/classes/sun/reflect/InstantiationExceptionConstructorAccessorImpl.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/InstantiationExceptionConstructorAccessorImpl.java Fri Nov 28 16:17:30 2014 +0300 @@ -33,7 +33,7 @@ class InstantiationExceptionConstructorAccessorImpl extends ConstructorAccessorImpl { - private String message; + private final String message; InstantiationExceptionConstructorAccessorImpl(String message) { this.message = message;
--- a/src/share/classes/sun/reflect/Label.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/Label.java Fri Nov 28 16:17:30 2014 +0300 @@ -47,10 +47,10 @@ } // This won't work for more than one assembler anyway, so this is // unnecessary - ClassFileAssembler asm; - short instrBCI; - short patchBCI; - int stackDepth; + final ClassFileAssembler asm; + final short instrBCI; + final short patchBCI; + final int stackDepth; } private List<PatchInfo> patches = new ArrayList<>();
--- a/src/share/classes/sun/reflect/NativeConstructorAccessorImpl.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/NativeConstructorAccessorImpl.java Fri Nov 28 16:17:30 2014 +0300 @@ -32,7 +32,7 @@ afterward, switches to bytecode-based implementation */ class NativeConstructorAccessorImpl extends ConstructorAccessorImpl { - private Constructor<?> c; + private final Constructor<?> c; private DelegatingConstructorAccessorImpl parent; private int numInvocations;
--- a/src/share/classes/sun/reflect/NativeMethodAccessorImpl.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/NativeMethodAccessorImpl.java Fri Nov 28 16:17:30 2014 +0300 @@ -32,7 +32,7 @@ switches to bytecode-based implementation */ class NativeMethodAccessorImpl extends MethodAccessorImpl { - private Method method; + private final Method method; private DelegatingMethodAccessorImpl parent; private int numInvocations;
--- a/src/share/classes/sun/reflect/ReflectionFactory.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/ReflectionFactory.java Fri Nov 28 16:17:30 2014 +0300 @@ -50,9 +50,9 @@ public class ReflectionFactory { private static boolean initted = false; - private static Permission reflectionFactoryAccessPerm + private static final Permission reflectionFactoryAccessPerm = new RuntimePermission("reflectionFactoryAccess"); - private static ReflectionFactory soleInstance = new ReflectionFactory(); + private static final ReflectionFactory soleInstance = new ReflectionFactory(); // Provides access to package-private mechanisms in java.lang.reflect private static volatile LangReflectAccess langReflectAccess;
--- a/src/share/classes/sun/reflect/SignatureIterator.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/SignatureIterator.java Fri Nov 28 16:17:30 2014 +0300 @@ -28,7 +28,7 @@ /** Assists in iterating down a method's signature */ public class SignatureIterator { - private String sig; + private final String sig; private int idx; public SignatureIterator(String sig) {
--- a/src/share/classes/sun/reflect/generics/reflectiveObjects/GenericArrayTypeImpl.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/reflectiveObjects/GenericArrayTypeImpl.java Fri Nov 28 16:17:30 2014 +0300 @@ -34,7 +34,7 @@ */ public class GenericArrayTypeImpl implements GenericArrayType { - private Type genericComponentType; + private final Type genericComponentType; // private constructor enforces use of static factory private GenericArrayTypeImpl(Type ct) {
--- a/src/share/classes/sun/reflect/generics/reflectiveObjects/ParameterizedTypeImpl.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/reflectiveObjects/ParameterizedTypeImpl.java Fri Nov 28 16:17:30 2014 +0300 @@ -38,9 +38,9 @@ /** Implementing class for ParameterizedType interface. */ public class ParameterizedTypeImpl implements ParameterizedType { - private Type[] actualTypeArguments; - private Class<?> rawType; - private Type ownerType; + private final Type[] actualTypeArguments; + private final Class<?> rawType; + private final Type ownerType; private ParameterizedTypeImpl(Class<?> rawType, Type[] actualTypeArguments,
--- a/src/share/classes/sun/reflect/generics/scope/DummyScope.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/scope/DummyScope.java Fri Nov 28 16:17:30 2014 +0300 @@ -38,7 +38,7 @@ public class DummyScope implements Scope { // Caches the unique instance of this class; instances contain no data // so we can use the singleton pattern - private static DummyScope singleton = new DummyScope(); + private static final DummyScope singleton = new DummyScope(); // constructor is private to enforce use of factory method private DummyScope(){}
--- a/src/share/classes/sun/reflect/generics/tree/ArrayTypeSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/ArrayTypeSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -28,7 +28,7 @@ import sun.reflect.generics.visitor.TypeTreeVisitor; public class ArrayTypeSignature implements FieldTypeSignature { - private TypeSignature componentType; + private final TypeSignature componentType; private ArrayTypeSignature(TypeSignature ct) {componentType = ct;}
--- a/src/share/classes/sun/reflect/generics/tree/BooleanSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/BooleanSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -29,7 +29,7 @@ /** AST that represents the type boolean. */ public class BooleanSignature implements BaseType { - private static BooleanSignature singleton = new BooleanSignature(); + private static final BooleanSignature singleton = new BooleanSignature(); private BooleanSignature(){}
--- a/src/share/classes/sun/reflect/generics/tree/BottomSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/BottomSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -28,7 +28,7 @@ import sun.reflect.generics.visitor.TypeTreeVisitor; public class BottomSignature implements FieldTypeSignature { - private static BottomSignature singleton = new BottomSignature(); + private static final BottomSignature singleton = new BottomSignature(); private BottomSignature(){}
--- a/src/share/classes/sun/reflect/generics/tree/ByteSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/ByteSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -29,7 +29,7 @@ /** AST that represents the type byte. */ public class ByteSignature implements BaseType { - private static ByteSignature singleton = new ByteSignature(); + private static final ByteSignature singleton = new ByteSignature(); private ByteSignature(){}
--- a/src/share/classes/sun/reflect/generics/tree/CharSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/CharSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -29,7 +29,7 @@ /** AST that represents the type char. */ public class CharSignature implements BaseType { - private static CharSignature singleton = new CharSignature(); + private static final CharSignature singleton = new CharSignature(); private CharSignature(){}
--- a/src/share/classes/sun/reflect/generics/tree/ClassTypeSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/ClassTypeSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -33,7 +33,7 @@ * AST representing class types. */ public class ClassTypeSignature implements FieldTypeSignature { - private List<SimpleClassTypeSignature> path; + private final List<SimpleClassTypeSignature> path; private ClassTypeSignature(List<SimpleClassTypeSignature> p) {
--- a/src/share/classes/sun/reflect/generics/tree/DoubleSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/DoubleSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -29,7 +29,7 @@ /** AST that represents the type double. */ public class DoubleSignature implements BaseType { - private static DoubleSignature singleton = new DoubleSignature(); + private static final DoubleSignature singleton = new DoubleSignature(); private DoubleSignature(){}
--- a/src/share/classes/sun/reflect/generics/tree/FloatSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/FloatSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -29,7 +29,7 @@ /** AST that represents the type float. */ public class FloatSignature implements BaseType { - private static FloatSignature singleton = new FloatSignature(); + private static final FloatSignature singleton = new FloatSignature(); private FloatSignature(){}
--- a/src/share/classes/sun/reflect/generics/tree/FormalTypeParameter.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/FormalTypeParameter.java Fri Nov 28 16:17:30 2014 +0300 @@ -29,8 +29,8 @@ /** AST that represents a formal type parameter. */ public class FormalTypeParameter implements TypeTree { - private String name; - private FieldTypeSignature[] bounds; + private final String name; + private final FieldTypeSignature[] bounds; private FormalTypeParameter(String n, FieldTypeSignature[] bs) { name = n;
--- a/src/share/classes/sun/reflect/generics/tree/IntSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/IntSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -29,7 +29,7 @@ /** AST that represents the type int. */ public class IntSignature implements BaseType { - private static IntSignature singleton = new IntSignature(); + private static final IntSignature singleton = new IntSignature(); private IntSignature(){}
--- a/src/share/classes/sun/reflect/generics/tree/LongSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/LongSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -29,7 +29,7 @@ /** AST that represents the type long. */ public class LongSignature implements BaseType { - private static LongSignature singleton = new LongSignature(); + private static final LongSignature singleton = new LongSignature(); private LongSignature(){}
--- a/src/share/classes/sun/reflect/generics/tree/MethodTypeSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/MethodTypeSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -28,10 +28,10 @@ import sun.reflect.generics.visitor.Visitor; public class MethodTypeSignature implements Signature { - private FormalTypeParameter[] formalTypeParams; - private TypeSignature[] parameterTypes; - private ReturnType returnType; - private FieldTypeSignature[] exceptionTypes; + private final FormalTypeParameter[] formalTypeParams; + private final TypeSignature[] parameterTypes; + private final ReturnType returnType; + private final FieldTypeSignature[] exceptionTypes; private MethodTypeSignature(FormalTypeParameter[] ftps, TypeSignature[] pts,
--- a/src/share/classes/sun/reflect/generics/tree/ShortSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/ShortSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -29,7 +29,7 @@ /** AST that represents the type short. */ public class ShortSignature implements BaseType { - private static ShortSignature singleton = new ShortSignature(); + private static final ShortSignature singleton = new ShortSignature(); private ShortSignature(){}
--- a/src/share/classes/sun/reflect/generics/tree/SimpleClassTypeSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/SimpleClassTypeSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -28,9 +28,9 @@ import sun.reflect.generics.visitor.TypeTreeVisitor; public class SimpleClassTypeSignature implements FieldTypeSignature { - private boolean dollar; - private String name; - private TypeArgument[] typeArgs; + private final boolean dollar; + private final String name; + private final TypeArgument[] typeArgs; private SimpleClassTypeSignature(String n, boolean dollar, TypeArgument[] tas) { name = n;
--- a/src/share/classes/sun/reflect/generics/tree/TypeVariableSignature.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/TypeVariableSignature.java Fri Nov 28 16:17:30 2014 +0300 @@ -28,7 +28,7 @@ import sun.reflect.generics.visitor.TypeTreeVisitor; public class TypeVariableSignature implements FieldTypeSignature { - private String identifier; + private final String identifier; private TypeVariableSignature(String id) {identifier = id;}
--- a/src/share/classes/sun/reflect/generics/tree/VoidDescriptor.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/generics/tree/VoidDescriptor.java Fri Nov 28 16:17:30 2014 +0300 @@ -30,7 +30,7 @@ /** AST that represents the pseudo-type void. */ public class VoidDescriptor implements ReturnType { - private static VoidDescriptor singleton = new VoidDescriptor(); + private static final VoidDescriptor singleton = new VoidDescriptor(); private VoidDescriptor(){}
--- a/src/share/classes/sun/reflect/misc/MethodUtil.java Thu Nov 27 16:20:25 2014 +0300 +++ b/src/share/classes/sun/reflect/misc/MethodUtil.java Fri Nov 28 16:17:30 2014 +0300 @@ -76,9 +76,9 @@ * Create a trampoline class. */ public final class MethodUtil extends SecureClassLoader { - private static String MISC_PKG = "sun.reflect.misc."; - private static String TRAMPOLINE = MISC_PKG + "Trampoline"; - private static Method bounce = getTrampoline(); + private static final String MISC_PKG = "sun.reflect.misc."; + private static final String TRAMPOLINE = MISC_PKG + "Trampoline"; + private static final Method bounce = getTrampoline(); private MethodUtil() { super();
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/java/lang/reflect/Generics/ThreadSafety.java Fri Nov 28 16:17:30 2014 +0300 @@ -0,0 +1,129 @@ +/* + * Copyright 2014 Google 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 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 8062771 8016236 + * @summary Test publication of Class objects via a data race + * @run testng ThreadSafety + */ + +import java.net.URL; +import java.net.URLClassLoader; +import java.util.Collections; +import java.util.concurrent.BrokenBarrierException; +import java.util.concurrent.Callable; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeoutException; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.testng.Assert.*; +import org.testng.annotations.Test; + +/** + * A test resulting from an attempt to repro this failure (in guice): + * + * java.lang.NullPointerException + * at sun.reflect.generics.visitor.Reifier.visitClassTypeSignature(Reifier.java:125) + * at sun.reflect.generics.tree.ClassTypeSignature.accept(ClassTypeSignature.java:49) + * at sun.reflect.generics.repository.ClassRepository.getSuperclass(ClassRepository.java:84) + * at java.lang.Class.getGenericSuperclass(Class.java:692) + * at com.google.inject.TypeLiteral.getSuperclassTypeParameter(TypeLiteral.java:99) + * at com.google.inject.TypeLiteral.<init>(TypeLiteral.java:79) + * + * However, as one would expect with thread safety problems in reflection, these + * are very hard to reproduce. This very test has never been observed to fail, + * but a similar test has been observed to fail about once in 2000 executions + * (about once every 6 CPU-hours), in jdk7 only. It appears to be fixed in jdk8+ by: + * + * 8016236: Class.getGenericInterfaces performance improvement. + * (by making Class.genericInfo volatile) + */ +public class ThreadSafety { + public static class EmptyClass { + public static class EmptyGenericSuperclass<T> {} + public static class EmptyGenericSubclass<T> extends EmptyGenericSuperclass<T> {} + } + + /** published via data race */ + private Class<?> racyClass = Object.class; + + private URL[] urls = ((URLClassLoader) ThreadSafety.class.getClassLoader()).getURLs(); + + private Class<?> createNewEmptyGenericSubclassClass() throws Exception { + URLClassLoader ucl = new URLClassLoader(urls, null); + return Class.forName("ThreadSafety$EmptyClass$EmptyGenericSubclass", true, ucl); + } + + @Test + public void testRacy_getGenericSuperclass() throws Exception { + final int nThreads = 10; + final int iterations = 30; + final int timeout = 10; + final CyclicBarrier newCycle = new CyclicBarrier(nThreads); + final Callable<Void> task = new Callable<Void>() { + public Void call() throws Exception { + for (int i = 0; i < iterations; i++) { + final int threadId; + try { + threadId = newCycle.await(timeout, SECONDS); + } catch (BrokenBarrierException e) { + return null; + } + for (int j = 0; j < iterations; j++) { + // one thread publishes the class object via a data + // race, for the other threads to consume. + if (threadId == 0) { + racyClass = createNewEmptyGenericSubclassClass(); + } else { + racyClass.getGenericSuperclass(); + } + } + } + return null; + }}; + + final ExecutorService pool = Executors.newFixedThreadPool(nThreads); + try { + for (Future<Void> future : + pool.invokeAll(Collections.nCopies(nThreads, task))) { + try { + future.get(iterations * timeout, SECONDS); + } catch (ExecutionException e) { + // ignore "collateral damage" + if (!(e.getCause() instanceof BrokenBarrierException) + && + !(e.getCause() instanceof TimeoutException)) { + throw e; + } + } + } + } finally { + pool.shutdownNow(); + assertTrue(pool.awaitTermination(2 * timeout, SECONDS)); + } + } +}